llvm dev - Oct 2015 - [AMDGPU] AMDGPUAsmParser fails to parse several instructions

Thanks you. I'm new to LLVM backend, so the help is much appreciated.

On Sat, Oct 24, 2015 at 2:12 AM, Matt Arsenault <arsenm2 at gmail.com>
wrote:
>
> > On Oct 23, 2015, at 3:36 AM, 李弘宇 via llvm-dev <llvm-dev at
lists.llvm.org>
> wrote:
>
> > The first line has the following error message:
> >
> > sop1-playground.s:1:15: error: invalid immediate: only 32-bit values
are
> legal
> > s_mov_b32 s0, 0xfe5163ab
>
> I’ve fixed this problem in  r251132.
>
Thanks for the reply and the quick fix of negative 32-bit immediate.
> >
> > The second line of the assembly has the assertion fail:
> >
> > llvm-mc:
>
/mnt/dm-0/codebase/Compilers/LLVM/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:184:
> void (anonymous namespace)::AMDGPUOperand::setModifiers(unsigned int):
> Assertion `isReg()' failed.
> >
> > and reports that 0.5 is "error: invalid operand for
instruction"
> >
> > v_mad_f32 v9, 0.5, v5, -v8
>
> There is a bug with operand parsing when you have immediate operands and
> source modifiers. I haven’t fixed this yet, but you can work around it for
> now by not using the source modifier.
>
I have no idea what to do the work-around. To me the modifier is set when
the register has absolute or negate. I traced the code and thought that the
problem occurs in the for loop to empty the modifiers for each source (in
the AMDGPUAsmParser::parseOperand around line 1040). If I add an if
statement like

if(RegOp.isInlineImm()) // because 0.5 is this case
    contiune;

before the statement to empty the modifier (RegOp.setModifiers(0);),

it obvious does not work and cause the other matching fails. Does it mean
to modify other files in other places, such as the VOP3Inst in the
SIInstrInfo.td, or is this nothing to do with the MatchOperandParserImpl
method or the other tablegen'd files.

I'll be grateful for any idea you might give me to work around this.

Regards,

李弘宇 (Li, Hong-Yu)
Department of Computer Science & Information Engineering
National Taiwan University

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151024/67e046f0/attachment.html>

> On Oct 24, 2015, at 10:50 AM, 李弘宇 <zhenlinospirit at gmail.com>
wrote:
> 
> I have no idea what to do the work-around. To me the modifier is set when
the register has absolute or negate.
Yes, this is what the modifiers are.
> I traced the code and thought that the problem occurs in the for loop to
empty the modifiers for each source (in the AMDGPUAsmParser::parseOperand around
line 1040). If I add an if statement like
> 
> if(RegOp.isInlineImm()) // because 0.5 is this case
>     contiune;
> 
> before the statement to empty the modifier (RegOp.setModifiers(0);), 
> 
> it obvious does not work and cause the other matching fails. Does it mean
to modify other files in other places, such as the VOP3Inst in the
SIInstrInfo.td, or is this nothing to do with the MatchOperandParserImpl method
or the other tablegen'd files.
> 
> I'll be grateful for any idea you might give me to work around this.
> 
> Regards,

The workaround is to either move the 0.5 into a register and use or to apply the
negate to the other operand first, e.g.:

v_mov_b32 v0, 0.5
v_mad_f32 v9, v0, v5, -v8

which will have a smaller encoding than the alternative of doing the fneg first.

The crash is easy enough to fix by checking isReg() before the setModifiers, but
then the instruction is incorrectly rejected. I’m not really sure what’s going
on with that right now, but I’m guessing it has to do with the way the modifiers
are part of the same parsed operand, but separate in the MCInst’s operands and
the missing modifier operand isn’t being added for immediates. I’m not really
sure the right way to fix this.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151024/9245ed65/attachment.html>

> On Oct 24, 2015, at 11:17 AM, Matt Arsenault <arsenm2 at gmail.com>
wrote:
> 
> The crash is easy enough to fix by checking isReg() before the
setModifiers, but then the instruction is incorrectly rejected. I’m not really
sure what’s going on with that right now, but I’m guessing it has to do with the
way the modifiers are part of the same parsed operand, but separate in the
MCInst’s operands and the missing modifier operand isn’t being added for
immediates. I’m not really sure the right way to fix this.
It seems it’s being rejected here in AMDGPUGenAsmMatcher.inc

  // 'RegWithInputMods' class
  if (Kind == MCK_RegWithInputMods) {
    if (Operand.isRegWithInputMods())
      return MCTargetAsmParser::Match_Success;
  }

Operand is actually an immediate. I would expect this to be reaching 

  // 'VCSrc32' class
  if (Kind == MCK_VCSrc32) {
    if (Operand.isVCSrc32())
      return MCTargetAsmParser::Match_Success;
  }

and passing here, but for some reason it isn’t.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151024/d1b6c859/attachment.html>