vkd3d-shader/dxil: Fix two bugs in handling LLVM forward-referenced type/value pairs.
Merge request reports
Activity
- cmp = sm6_parser_get_value_by_ref(sm6, record, type, &i); + /* Load a type/value pair for a forward-referenced comparand. This corresponds + * to the use of getValueTypePair() instead of popValue() in the LLVM source. */ + cmp = sm6_parser_get_value_by_ref(sm6, record, NULL, &i);
That's not the main thing I'd like to know when reading the commit though. Why is passing "type" here the wrong thing to do? In what kind of cases does is break?
That's not the main thing I'd like to know when reading the commit though. Why is passing "type" here the wrong thing to do? In what kind of cases does is break?
The word "obvious" is unfortunately more subjective than one would often like to think, but isn't that information rather "obvious", or at least simple to work out, when reading
sm6_parser_get_value_idx_by_ref()
? The comment even hints in that direction. Everybody has their own thought processes, of course, but reading that function makes it pretty evident to me that setting or not setting that parameter is going to change the number of DXIL operands that get consumed (if the reference is inedeed a forward reference), so it's quite likely that either it must be set or it must not.We could certainly decide that in general our code is less commented than it should be (and to be honest sometimes I'm tempted to start working on patches that do nothing but add comments to code that already exists), but honestly I wouldn't mention this as a glaring example of that. That said, it's certainly legitimate to ask for more clarity.
mentioned in merge request !1102 (closed)
added 16 commits
-
85b71907...95bd06be - 14 commits from branch
wine:master
- 6a86e1a8 - vkd3d-shader/dxil: Load forward-referenced comparands as value/type pairs in...
- 2bf51778 - vkd3d-shader/dxil: Load forward-referenced sources as value/type pairs in sm6_parser_emit_store().
-
85b71907...95bd06be - 14 commits from branch