[PATCH] [PATCH][ARM] Fix issue with UMLAL (unsigned multiple accumulate long) lowering

[Matthias Braun]

> mul r2, r1, r3

>  SelectionDAG::computeKnownBits does not return the bit info for this instruction ISD::MUL if the mul operands are of opcode ISD::CopyFromReg This is not handled in computeKnownBits. I tried to do that but, RegisterSDNode class only gives register number, not the value contained in the register. How could one access the value contained in a RegisterSDNode, so as to check that MUL operands have their upper 16bit known to be zero ?


It's possible that the computeKnownBits doesn't catch all cases yet - or maybe CopyFromReg can't guarantee that in general, not sure.

> I also tried,

>  MULOp.getOperand(0).getValueSizeInBits() > 16 = overflow

>  but MULOp.getOperand(0).getValueSizeInBits() always returns 32 irrespective of the value stored in MUL operands.

> 

> This is my code to check overflow

> 

>   MULOp = (AddcOp0->getOpcode() == ISD::MUL) ? AddcOp0 : AddcOp1;

>   APInt KnownZero, KnownOne;

>       APInt KnownZero1, KnownOne1;

>       DCI.DAG.computeKnownBits(MULOp.getOperand(0), KnownZero, KnownOne);

>       DCI.DAG.computeKnownBits(MULOp.getOperand(1), KnownZero1, KnownOne1);

>   

>     APInt Mask(APInt::getHighBitsSet(VT.getSizeInBits(), 16));

>     if ((KnownZero & Mask).getBoolValue() && (KnownZero1 & Mask).getBoolValue())

>         return SDValue();

>   

> 

> 

> This should do the trick, but KnownZero is always zero even when MUL operands contain 0xFFFFFFFF, 0XFFFFFFFF for reasons sited above.

>  I would highly appreciate your inputs on this.


This needs to have || in the if condition, but apart from that looks okay to me.

I tried your original example again and ToT clang gets met this:

  %0 = load i8, i8* %b, align 1, !tbaa !2
  %conv = zext i8 %0 to i64
  %1 = load i16, i16* %c, align 2, !tbaa !5
  %conv1 = zext i16 %1 to i64
  %mul = mul nuw nsw i64 %conv1, %conv
  %add = add i64 %mul, %a
  ret i64 %add

So the mul operation already has an NUW flag that should be preserved, you should be able to check it by casting to OverflowingBinaryOperator and checking hasNoUnsignedWrap(). Thinking about it some more that is probably the best solution anyway as the IR optimizations already do computeKnownBits tricks to determine that flag, so it's probably best to just check for that flag and not do anything more fancy.


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9881

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/