I may be completely off on this one, please bear with me :)
The way we currently define congruency for two PHIExpression(s) is:
- The operands to the phi functions are congruent
- The PHIs are defined in the same Basic Block.
The attached testcase contains the following:
patatino: %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ] %banana = phi i16 [ %0, %tinky ], [ %conv1, %winky ] br label %end
Looking at the NewGVN debug we believe that %meh and %banana are congruent. I don't think they are, in fact this causes a miscompile (you can run the following on the testcase).
Processing instruction %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ] Created new congruence class for %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ] using expression { ExpressionTypePhi, opcode = 53, operands = {[0] = i16 %0 [1] = i16 %conv1 } bb = 0x486b1e0} at 10 and leader %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ] New class 10 for expression { ExpressionTypePhi, opcode = 53, operands = {[0] = i16 %0 [1] = i16 %conv1 } bb = 0x486b1e0} Processing instruction %banana = phi i16 [ %0, %tinky ], [ %conv1, %winky ] New class 10 for expression { ExpressionTypePhi, opcode = 53, operands = {[0] = i16 %0 [1] = i16 %conv1 } bb = 0x486b1e0}
[davide@cupiditate bin]$ ./opt -newgvn phi.ll | ./lli 65535 65535 [davide@cupiditate bin]$ ./opt -gvn phi.ll | ./lli 0 65535
I'm under the impression the problem is that we don't take in consideration the incoming edges for congruency. This patch makes sure we only consider congruent phis if the incoming BBs are the same.
If we shouldn't, I'll appreciate suggestions on how to fix alternatively :)
This would actually be wrong.
They are congruent if the same incoming bbs have the same arguments ,and for a given block, the set of incomingbbs is always the same.
The only issue here is the order of the incoming bbs.