[IA][THUMB] warn when addressing mode not supported by current mode

[Quuxplusone]

Bugzilla Link	PR38505
Status	NEW
Importance	P enhancement
Reported by	Nick Desaulniers (ndesaulniers@google.com)
Reported on	2018-08-09 13:32:23 -0700
Last modified on	2018-08-14 06:42:56 -0700
Version	trunk
Hardware	PC Linux
CC	echristo@gmail.com, kristof.beyls@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@ndave.org, oliver.stannard@arm.com, pirama@google.com, srhines@google.com
Fixed by commit(s)
Attachments	memcpy_minimal.S (183 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 20666
memcpy_minimal.S

See the attached file for reproduction steps.

What's curious is that clang produces the warning:

 error: too many operands for instruction

for the first instance but not the second.  Also, the gas errors are a little
more helpful:

memcpy_minimal.S:11: Error: Thumb does not support this addressing mode -- `ldr
r3,[r1],#4'

[Quuxplusone]

Attached memcpy_minimal.S (183 bytes, text/plain): memcpy_minimal.S

[Quuxplusone]

Ah, a coworker noted that in thumb mode, ldr == 16b instruction and ldr.w == 32b instruction.

So maybe a better title+focus for this bug is to warn when an instruction prefix is not supported in the current mode?

[Quuxplusone]

Sorry, reviewing this again, it seems that this test case should be supported. Maybe the post-index case is not supported?

[Quuxplusone]

Warning seems to be coming from lib/Target/ARM/AsmParser/ARMAsmParser.cpp.

[Quuxplusone]

lib/Target/ARM/ARMInstrThumb2.td seems to have an entry for the post index load:

1322 def t2LDR_POST : T2Ipostldst<0, 0b10, 1, 0, (outs GPR:$Rt, GPR:$Rn_wb),
1323                           (ins addr_offset_none:$Rn,
t2am_imm8_offset:$offset),
1324                           AddrModeT2_i8, IndexModePost, IIC_iLoad_iu,
1325                           "ldr", "\t$Rt, $Rn$offset", "$Rn = $Rn_wb", []>,
1326                   Sched<[WriteLd]>;

I think this is a case similar to commit e001a156b8df ("[ARM] Add .w aliases of
MOV with shifted operand") and just needs to be extended to ldr.

[Quuxplusone]

It looks like adding the .w aliases will fix this case, but I wonder if there is a better way to solve this than adding aliases for every 32-bit Thumb instruction.

The current system also has the problem that it just throws away the '.n' width specifier, so we don't diagnose instructions with '.n' which end up assembling to a 32-bit instruction.

I've done some experiments with tracking the .n/.w specifier as a separate check in the asm parser, rather than as an operand, which allows us to remove all of the existing aliases, but there are currently a lot of regression test failures, I'll need to go through these and check if they are OK of if there is a problem with this idea.

If that doesn't work, an alternative would be to work out a way to exhaustively test instructions with and without the '.w' specifier, to make sure that we have aliases for every instruction which needs them.