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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

  diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
  index ca56e8b9328c..50ece4b65990 100644
  --- a/llvm/include/llvm/IR/Constants.h
  +++ b/llvm/include/llvm/IR/Constants.h
  @@ -154,7 +154,7 @@ public:
     /// this method can assert if the value does not fit in 64 bits.
     /// Return the sign extended value.
     inline int64_t getSExtValue() const {
  -    return Val.getSExtValue();
  +    return (getBitWidth() == 1) ? Val.getZExtValue() : Val.getSExtValue();
     }
   
     /// A helper method that can be used to determine if the constant contained

is what I meant.  Produces 23 test failures (I didn't try @kees tests from this patch):

  LLVM :: CodeGen/AMDGPU/cf-loop-on-constant.ll
  LLVM :: CodeGen/AMDGPU/divergent-branch-uniform-condition.ll
  LLVM :: CodeGen/AMDGPU/function-args.ll
  LLVM :: CodeGen/AMDGPU/i1-copy-from-loop.ll
  LLVM :: CodeGen/AMDGPU/i1-copy-phi-uniform-branch.ll
  LLVM :: CodeGen/AMDGPU/i1-copy-phi.ll
  LLVM :: CodeGen/AMDGPU/infinite-loop.ll
  LLVM :: CodeGen/AMDGPU/inline-asm.ll
  LLVM :: CodeGen/AMDGPU/llvm.amdgcn.div.fmas.ll
  LLVM :: CodeGen/AMDGPU/llvm.amdgcn.kill.ll
  LLVM :: CodeGen/AMDGPU/llvm.amdgcn.ps.live.ll
  LLVM :: CodeGen/AMDGPU/loop_break.ll
  LLVM :: CodeGen/AMDGPU/loop_exit_with_xor.ll
  LLVM :: CodeGen/AMDGPU/multi-divergent-exit-region.ll
  LLVM :: CodeGen/AMDGPU/multilevel-break.ll
  LLVM :: CodeGen/AMDGPU/sub_i1.ll
  LLVM :: CodeGen/AMDGPU/trunc-cmp-constant.ll
  LLVM :: CodeGen/AMDGPU/valu-i1.ll
  LLVM :: CodeGen/PowerPC/fast-isel-ret.ll
  LLVM :: CodeGen/X86/avx512-fsel.ll
  LLVM :: CodeGen/X86/pr32256.ll
  LLVM :: CodeGen/X86/pr32284.ll

Seems like the test cases should probably be updated (they all seem to have `-1`s converted to `1`s which I don't think the original `-1`s were intentional)?  I'd probably also add an explicitly named method to `ConstantInt` named `SExtBoolean` or something that had the original semantics if you truly meant to sign extend a boolean, because chances are, you dont.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60224