[PATCH] D78319: [GlobalISel][InlineAsm] Add support for basic input operand constraints

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:451
+                          << OpInfo.ConstraintCode << "'\n");
+
+        return false;
----------------
kschwarz wrote:
> arsenm wrote:
> > Extra newline
> Is there a way to catch these formatting issues with clang-format? It doesn't remove those superfluous newlines for me 
I think it just doesn't implement this


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:574-575
+      LLT Ty = getLLTForType(*Val->getType(), MIRBuilder.getDataLayout());
+      unsigned ExtOpc =
+          Ty.isByteSized() ? TargetOpcode::G_SEXT : TargetOpcode::G_ZEXT;
+      int64_t ExtVal = ExtOpc == TargetOpcode::G_SEXT ? CI->getSExtValue()
----------------
kschwarz wrote:
> arsenm wrote:
> > kschwarz wrote:
> > > arsenm wrote:
> > > > Changing the extension type based on the byte-sizedness doesn't really make sense to me
> > > I admit I simply followed what is done in the legalizer for extending constants (https://reviews.llvm.org/D70922).
> > > 
> > > Do you have a suggestion how the condition should look like instead? 
> > I think we generally just sign extend immediate operands like this. There's no legalization or register here, so you shouldn't even need to look at the LLT
> Yes, but it looks like inline asm boolean constants should behave differently (https://reviews.llvm.org/D60224)?
That seems convoluted. That's at least checking explicitly for a bit width of 1, not isByteSized


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78319