[llvm-commits] [llvm] r123034 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineSelect.cpp test/Transforms/InstCombine/select.ll
Frits van Bommel
fvbommel at gmail.com
Sun Jan 9 03:17:05 CST 2011
On Sun, Jan 9, 2011 at 9:04 AM, Tobias Grosser
<grosser at fim.uni-passau.de> wrote:
> On 01/09/2011 01:46 AM, Cameron Zwarich wrote:
>> Even with r123061, I still see the following on one of my tests against
>> SPEC. I guess I should make a reduction for you guys.
> I believe the attached patch should fix the issue. Could you give it a try?
> I am not 100% sure, if this is the right approach so a review before
> committing this would be appreciated.
- ICI->setOperand(0, CmpLHS);
- ICI->setOperand(1, CmpRHS);
+ // Move ICI instruction right before the select instruction. Otherwise
+ // the sext/zext value may be defined after the ICI instruction uses
+ // it.
+ ICmpInst *newICI = new ICmpInst(&SI, Pred, CmpLHS, CmpRHS);
+ ReplaceInstUsesWith(*ICI, newICI);
+ StringRef ICIName = ICI->getName();
Instead of creating a whole new ICmp in a new place and deleting the
old one, you can keep the updating code and use ICI->moveBefore(SI).
This should have the same effect but be much cheaper.
For future reference: Value::takeName() is probably more efficient
than getName() + setName().
In the comment above this you should probably replace "edit ICI" with
"move ICI or edit it".
The only case I can think of where moving the icmp might be
undesirable would be if it was a loop-invariant instruction and the
extension operation already dominated it (or it would be cheaper to
move/copy the extension to just before the icmp). I don't think this
is a very important case, especially since the first few runs of
-instcombine are followed by loop passes that should clean this up.
P.S. While I can see how the sext/zext might not dominate the icmp, I
wonder how it's possible that some of those errors were for icmp
conditions not dominating selects since this code never touches the
condition operand of the select...
More information about the llvm-commits