[llvm-commits] [llvm] r124534 - in /llvm/trunk: include/llvm/Analysis/InstructionSimplify.h lib/Analysis/InstructionSimplify.cpp lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineMulDivRem.cpp test/Transforms/InstSimp

Frits van Bommel fvbommel at gmail.com
Sat Jan 29 09:57:17 PST 2011


On Sat, Jan 29, 2011 at 6:06 PM, Duncan Sands <baldrick at free.fr> wrote:
>> +static Value *SimplifyFDivInst(Value *Op0, Value *Op1, const TargetData *TD,
>> +                               const DominatorTree *DT, unsigned MaxRecurse) {
>
> since none of TD, DT or MaxRecurse are used, some compilers will issue warnings.
> The fix is to not to give these parameters names.

I copied this from SimplifyUDivInst which actually uses it's parameters.
However, when I pointed the same thing out to Chris about a week ago
he said that wasn't the case for parameters:
<http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110117/115614.html>.
And when I compile (with make+cmake+gcc/clang on Linux), there's a
"-Wno-unused-parameter" on the command line to disable warnings for
unused parameters.

Chris didn't fix it for r124066 and nobody seems to have noticed...

>> -Instruction *InstCombiner::visitFDiv(BinaryOperator&I) {
>> -  Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
>> -
>> -  // undef / X ->  undef    (the undef could be a snan).
>> -  if (isa<UndefValue>(Op0))
>> -    return ReplaceInstUsesWith(I, Op0);
>> -
>> -  // X / undef ->  undef
>> -  if (isa<UndefValue>(Op1))
>> -    return ReplaceInstUsesWith(I, Op1);
>> -
>> -  return 0;
>> -}
>
> You should have FDiv call SimplifyFDiv.  Currently instcombine doesn't
> automagically call SimplifyInstruction on everything (because it leads
> to regressions) so you have to do it in each place where you want it.
> For example, I expect your testcases to fail when using -instcombine

Indeed it does. How annoying.
I remembered instcombine calling SimplifyInstruction on everything
before anything else, but I must have missed the commit that removed
that.

Fixed this in r124535.




More information about the llvm-commits mailing list