Friedman, Eli via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 18 11:23:19 PST 2017
On 1/18/2017 6:13 AM, Jonas Paulsson via llvm-commits wrote:
> I wanted to add a custom handling of ISD::SIGN_EXTEND in
> ReplaceNodeResults(), so that
> a sext of v2i16->v2i32 would be done the same way as a v4i16 -> v4i32.
> First, I added
> setOperationAction(ISD::SIGN_EXTEND, MVT::v2i32, Custom);
> This all worked out well after a while...
> To my surprise though, another test case then failed, with a sext
> v2i32 -> v2i64. The fail
> is caused by the sext end up in LowerOperation(), where it isn't
> handled and so an assert
> This is quite weird, I think - to have the SIGN_EXTEND of *another
> type* than given with setOperationAction() all of a sudden get custom
> What happens is that in WidenVectorOperand(), this code:
> // See if the target wants to custom widen this node.
> if (CustomLowerNode(N, N->getOperand(OpNo).getValueType(), false))
> return false;
> checks for custom lowering a sext of v2i32, while N is actually a sext
> I can't imagine this to be right so I am guessing that usually the
> operands and results have the same value type, so this usually doesn't
> matter, and so this is in fact a bug?
This is a little confusing, but I wouldn't call it a bug. We
consistently call CustomLowerNode with the illegal type, whether it's
the result or an operand, and existing targets depend on that.
> On the other hand, with this patch plenty of regression tests in the
> X86 backend fail, so please help me out. (If this is the way the code
> must behave, I suppose then LowerOperation() must then return 'Op'
> unchanged when the sext we don't want to custom handle appears...?)
If you want the legalizer to use its default expansion for a node, you
should just return SDNode().
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
More information about the llvm-commits