[PATCH] Change the policy of ZERO_EXTEND/SIGN_EXTEND for SelectionDAG builder and legalization

Tim Northover t.p.northover at gmail.com
Wed Aug 20 02:22:36 PDT 2014


Hi Jiangning,

Thanks for the update. I think those asserts will be a problem though:

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:875
@@ +874,3 @@
+    if (OpL->getOpcode() == ISD::AssertSext &&
+        NewLHS->getOpcode() == ISD::TRUNCATE &&
+        OpR->getOpcode() == ISD::AssertSext &&
----------------
Jiangning Liu wrote:
> Tim Northover wrote:
> > I think this can be generalized slightly. It looks like the check against TRUNCATE is really just guarding against us deciding to apply sign extension to (assertsext LHS, i16) when we're actually emitting an i8 setcc.
> > 
> > I'd suggest something like
> >     if (OpL->getOpcode() == ISD::AssertSext &&
> >         cast<VTSDNode>(OpL->getOperand(1))->getVT() == NewLHS.getValueType() &&
> >         [...]
> OK. I changed the code to be like this. But I think the only case is NewLHS and NEWRHS are TRUNCATE, so I added assertions inside the if body. Is there any other case except TRUNCATE?
Yes. In principle, it could come from any DAG combine or other inference (computeKnownBits for example) that inspected the input.

The simplest example I was able to exploit was PromoteIntRes_FP_TO_XINT. This program crashes with the asserts present:
    define i1 @foo(float %inl, float %inr) {
      %lval = fptosi float %inl to i8
      %rval = fptosi float %inr to i8
      %sum = icmp eq i8 %lval, %rval
      ret i1 %sum
    }

http://reviews.llvm.org/D4967






More information about the llvm-commits mailing list