[PATCH] D99635: [SelectionDAG] Add extra check on asm operand legalization.

[Michael Liao via Phabricator via llvm-commits]

hliao added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8190-8191
          OpInfo.Type == InlineAsm::isInput) &&
-        !TRI.isTypeLegalForClass(*RC, OpInfo.ConstraintVT)) {
+        (!TLI.isTypeLegal(OpInfo.ConstraintVT) ||
+         !TRI.isTypeLegalForClass(*RC, OpInfo.ConstraintVT))) {
       // Try to convert to the first EVT that the reg class contains.  If the
----------------
arsenm wrote:
> hliao wrote:
> > arsenm wrote:
> > > I'm not sure I see what case this fixes exactly. The concept of type legality is so convoluted already.
> > this change fixes the issue shown in `llvm/test/CodeGen/AMDGPU/inline-asm.i128.ll`. I was working on a similar issue reported but found your previous change removing i128 as a legal type.
> So the point is to re-add i128 as a legal type?
> 
> I think I see where this is going, but it's hack on a hack. We want the type to occupy the registers, but we can't make them legal since we can't opt out of the consequences of doing so. This at least needs a comment explaining this
But, there we really should check `TLI.isTypeLegal()` in addition to checking RC legality. The latter is specific to that inlined assembly and the former is target-wise. Both should be checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99635