[PATCH] D20907: [IfConversion] Bugfix: don't add Undef flag on use if reg is live.
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 6 15:16:24 PDT 2016
MatzeB added a comment.
In http://reviews.llvm.org/D20907#473455, @jonpa wrote:
> Matthias, from what I can see you have some experience with this part of IfConversion.cpp. Perhaps you could give some advice on or even fix this problem? In short, a bug in IfConversion was found and fixed, but when patch was applied, we ran into a problem relating to Redefs - see above.
> I am now on vacation, and cannot not fix IfConversion right now, so if anybody could take the time that would be great, as this is a bug that should be fixed ASAP.
It has been a while since I worked on the IfConversion pass.
Just to document the problem for myself and the record: The current way of modeling predicated instructions as a sequence of instructions is a bad fit to our expectations of how read/write operands work. We expect a read/write operand to always read/write the register which may not be true for a predicated instruction that isn't executed. We get by by cheating and adding enough def/use operands until liveness seemingly makes sense even if you are not aware of the predication effects. To give a typical example (I am prefixing the instructions with P0 and http://reviews.llvm.org/P1 to indicate two disjunct predicates):
0 %R1 = ...
10 P0: %R0 = ...
20 P0: %R1 = ...
30 P1: %R0 = %R1
40 = use %R0
50. = use %R1
The real situation here is:
a) `R1` is defined at instruction 0 and this definition is live at the beginning of instruction 30. `R1` however is not live at instruction 20, `R1` is live after instruction 20 however the value is different from the value live at instruction 0 and 30.
b) `R0` is live from instruction 10-40.
We cannot represent most of these subtleties and therefore "fixup" the representation with implicit operands in a way that we can ignore the predication:
a) Value `R1` cannot simply become dead without a killing use at instruction 10, we conservatively assume it is still alive instead. We can achieve this here by adding an implicit use operand on instruction 20 which would otherwise kill `R1`.
b) The `R0` definition at instruction 30 would make it look like `R0` is undefined so we add another undef use.
The IfConverter currently does those fixups which should be good enough to model the liveness correctly (in a conservative manner). I still consider this somewhat a hack as the relations between defs and uses are bogus after this conversion, we are merely saved by the fact that the IfConversion pass runs really late and we do not have any later passes looking into def/use relationships.
More information about the llvm-commits