LLVM legalizer crashes when target legalizes some operation and LLVM legalizes some

[Quuxplusone]

Bugzilla Link	PR28365
Status	NEW
Importance	P normal
Reported by	Hisham Chowdhury (hisham_chow@yahoo.com)
Reported on	2016-06-29 15:50:08 -0700
Last modified on	2016-06-29 16:21:06 -0700
Version	trunk
Hardware	Macintosh MacOS X
CC	hfinkel@anl.gov, hisham_chow@yahoo.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	LegalizeIntegerTypes.cpp.patch (4101 bytes, application/octet-stream) legalizationIssue.ll (837 bytes, application/octet-stream) LegalizeIntegerTypes.cpp.patch (4101 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 16650
patch

For example, when target marks i8, i16 type as illegal for the target, LLVM
backend legalization kicks in and tries to promote the operation to higher
supported types using “GetPromotedInteger” in LegalizeIntegerTypes.cpp->
PromoteIntegerOperand and updates a map (PromotedIntegers[Op]) to indicate the
values promoted and to what type. But this function doesn’t handle target
intrinsics which uses these illegal types. So, when target provides hooks to
legalize the target intrinsic, target promotes the node to higher type(in this
case i32) and return the result. That code path from PromoteIntegerOperand
function returns right away without updating the map when target provides the
legalization hook:
  // See if the target wants to custom expand this node.
  if (CustomLowerNode(N, N->getValueType(ResNo), true))
    return;
So when user(standard ISD instruction, example SLL) of the target intrinsic
comes in and when LLVM tries to legalize it by calling GetPromotedInteger-
>PromotedIntegers[Op], the code expecting the Op to be i16, but target already
promoted it to i32. So, the map return NULL and the code fails/asserts
Here is a simple code snippet that expose the issue:

  %1 = tail call zeroext i16 @llvm.igil.ftous.sat(float -1.000000e+00) #2 // target legalizes it and changes the result to i32
  %2 = shl i16 %1, 11
  %3 = zext i16 %2 to i32, !dbg !22

Looks like some of the path related to this issue is fixed in the trunk(shift
legalization case), but not all the functions that use GetPromotedInteger.
Here is an example of shift legalization fix:
#ifdef LATEST
    SDValue LHS = N->getOperand(0);
    SDValue RHS = N->getOperand(1);
      if (getTypeAction(LHS.getValueType()) == TargetLowering::TypePromoteInteger)
        LHS = GetPromotedInteger(LHS);
      if (getTypeAction(RHS.getValueType()) == TargetLowering::TypePromoteInteger)
        RHS = ZExtPromotedInteger(RHS);
      return DAG.getNode(ISD::SHL, SDLoc(N), LHS.getValueType(), LHS, RHS);
#else

  SDValue Res = GetPromotedInteger(N->getOperand(0));
  SDValue Amt = N->getOperand(1);
  Amt = Amt.getValueType().isVector() ? ZExtPromotedInteger(Amt) : Amt;
  return DAG.getNode(ISD::SHL, SDLoc(N), Res.getValueType(), Res, Amt);
#endif

Where the code inside #ifdef LATEST is the new code.

I hit the same kind of issue in “PromoteIntRes_SimpleIntBinOp” where trunk code
doesn’t have similar fix.

The attached patch expands that fix to other legalize op functions

Attachments:

[Quuxplusone]

Attached LegalizeIntegerTypes.cpp.patch (4101 bytes, application/octet-stream): patch

[Quuxplusone]

Attached legalizationIssue.ll (837 bytes, application/octet-stream): simple test to hit legalization issue

[Quuxplusone]

Attached LegalizeIntegerTypes.cpp.patch (4101 bytes, text/plain): fix patch