[PATCH] D60224: [TargetLowering] Extend bool args to inline-asm according to getBooleanType

Mon May 13 10:45:46 PDT 2019

nickdesaulniers added a comment.

ToT LLVM fails w/ all 9 new tests. (which is expected, and shows the value of the tests added here, which expose the bug).

================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3376
+        bool IsBool = C->getConstantIntValue()->getBitWidth() == 1;
+        BooleanContent BCont = getBooleanContents(MVT::i64);
+        ISD::NodeType ExtOpc = IsBool ? getExtendForContent(BCont)
----------------
nickdesaulniers wrote:
> efriedma wrote:
> > Is getBooleanContents really the predicate here?  Despite the name, it's not really an ABI hook; it's specifically supposed to describe the behavior of a few SelectionDAG instructions like ISD::SELECT.  I'd prefer to just say booleans are always zero-extended, unless we come across some case where gcc doesn't do that.
> Ok, sounds like ZExt'ing bools when asked to SExt is not the way to go.  We'll have to rebase this or https://reviews.llvm.org/D61560, depending on which lands first ;) . I agree that always Zextending is probably simpler while being correct.
> 
> IIUC, the code in the patch and the previous one this is based on only Zext if the boolean is not zero.
This patch needs rebasing now due to https://reviews.llvm.org/rL360604 (sorry).  Line 3563 should look familiar.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60224/new/

https://reviews.llvm.org/D60224