[NVPTX] Add a truncate DAG node to some calls.

[jlebar]

Summary

Previously, we were running afoul of the assertion

EVT(CLI.Ins[i].VT) == InVals[i].getValueType() && "LowerCall emitted a value with the wrong type!"

in SelectionDAGBuilder.cpp when running the NVPTX/i8-param.ll test.
This is because our backend (for some reason) treats small return values
as i32, but it wasn't ever truncating the i32 back down to the expected
width in the DAG.

Unclear to me whether this fixes any actual bugs -- in this test, at
least, the generated code is unchanged.

[jingyue]

lib/Target/NVPTX/NVPTXISelLowering.cpp

1641

Why shouldn't we set needTruncate here too?

[jlebar]

lib/Target/NVPTX/NVPTXISelLowering.cpp

1641

Interesting... I think this comment is wrong? If returning i1 or i8 (or i16), we should hit the first branch. This branch should be for *non-int* types of size less than 16? But then, I'm not sure what that is, so I'm not sure when we ever run this code.

[jingyue]

I am not clear with this part either. jholewinski@, can you comment on this?

[jlebar]

Justin, friendly ping.

[jholewinski]

That's for... hmm... I don't know. I don't see how that block would be executed. The call lowering code has always been a mess, unfortunately.

[jlebar]

In D17872#388029, @jholewinski wrote:

That's for... hmm... I don't know. I don't see how that block would be executed. The call lowering code has always been a mess, unfortunately.

Hey, a real, live Chesterton's Fence!

In the interest of not changing things that I don't understand, I think I'll leave out the needTruncate on this path, and add a comment saying we may need it, if we can figure out what the branch is for at all.

[jlebar]

Add comment about branch we don't understand.

[jlebar]

I would like to check in the one fix we know makes sense so that I can reenable the assertion we're hitting. (Currently you have to pass -debug to hit it, which is why we missed it in the first place.)

[jingyue]

Other than that FIXME, pretty straightforward. LGTM

lib/Target/NVPTX/NVPTXISelLowering.cpp

1621

Maybe call it BitWidth instead?

[jlebar]

lib/Target/NVPTX/NVPTXISelLowering.cpp

1621

I'll fix that and the variable capitalization here in a separate change, which I'll land right after this one.

[jlebar]

lib/Target/NVPTX/NVPTXISelLowering.cpp

1621

Actually, I take it back. The whole file is actually pretty consistent with "sz" and lower-case "align". The LLVM style guide says to be consistent and to avoid big refactorings unless you're touching all the code anyway. I think we should probably leave this sleeping dragon un-tickled for now.

lib/Target/NVPTX/NVPTXISelLowering.cpp

if (retTy && retTy->isVectorTy()) {
       }
     } else {
       SmallVector<EVT, 16> VTs;
       SmallVector<uint64_t, 16> Offsets;
       ComputePTXValueVTs(*this, DAG.getDataLayout(), retTy, VTs, &Offsets, 0);
       assert(VTs.size() == Ins.size() && "Bad value decomposition");
       unsigned RetAlign = getArgumentAlignment(Callee, CS, retTy, 0);
       for (unsigned i = 0, e = Ins.size(); i != e; ++i) {
         unsigned sz = VTs[i].getSizeInBits();
         unsigned AlignI = GreatestCommonDivisor64(RetAlign, Offsets[i]);
-        bool needTruncate = sz < 8;
-        if (VTs[i].isInteger() && (sz < 8))
+        bool needTruncate = false;
+        if (VTs[i].isInteger() && sz < 8) {
           sz = 8;
+          needTruncate = true;
+        }
 
         SmallVector<EVT, 4> LoadRetVTs;
         EVT TheLoadType = VTs[i];
         if (retTy->isIntegerTy() && DL.getTypeAllocSizeInBits(retTy) < 32) {
           // This is for integer types only, and specifically not for
           // aggregates.
           LoadRetVTs.push_back(MVT::i32);
           TheLoadType = MVT::i32;
+          needTruncate = true;
         } else if (sz < 16) {
           // If loading i1/i8 result, generate
           //   load i8 (-> i16)
           //   trunc i16 to i1/i8
           LoadRetVTs.push_back(MVT::i16);
         } else
           LoadRetVTs.push_back(Ins[i].VT);
         LoadRetVTs.push_back(MVT::Other);
         LoadRetVTs.push_back(MVT::Glue);
 
         SDValue LoadRetOps[] = {Chain, DAG.getConstant(1, dl, MVT::i32),
                                 DAG.getConstant(Offsets[i], dl, MVT::i32),
                                 InFlag};