[PATCH] D150068: [X86][AsmParser] Refactor code in AsmParser

[Kan Shengchen via Phabricator via llvm-commits]

skan marked an inline comment as done.
skan added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrAsmAlias.td:554
 
-/*  FIXME: This is disabled because the asm matcher is currently incapable of
- *  matching a fixed immediate like $1.
----------------
barannikov88 wrote:
> skan wrote:
> > barannikov88 wrote:
> > > I think this can be done by implementing `validateTargetOperandClass` for `MCK__36_1` ($1).
> > > See how ARM does this.
> > > 
> > Thanks for the info! I added the comments to `optimizeShiftRotateWithImmediateOne`.
> I tried to do what I suggested and it doesn't work for all cases.
> The "short" variants should be matched first, but this is not always happening.
> Currently there doesn't seem to be a way to force ordering between the variants.
> (AsmOperandClass has SuperClasses field that guarantees partial ordering, but there is no explicit operand to attach the class to.)
> 
> I tried to do what I suggested and it doesn't work for all cases.
> The "short" variants should be matched first, but this is not always happening.
> Currently there doesn't seem to be a way to force ordering between the variants.
> (AsmOperandClass has SuperClasses field that guarantees partial ordering, but there is no explicit operand to attach the class to.)
> 

It doesn't matter. As craig suggested, we may remove all the isel patterns that select the 1 immediate version by `optimizeShiftRotateWithImmediateOne`,  so `InstAlias` might be not a better direction.

But I still think the comment about `validateTargetOperandClass` is valuable b/c it provides a new perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150068