[llvm] r206669 - Change the ARM assembler to require a :lower16: or :upper16 on non-constant
Kevin Enderby
enderby at apple.com
Fri Apr 18 16:06:39 PDT 2014
Author: enderby
Date: Fri Apr 18 18:06:39 2014
New Revision: 206669
URL: http://llvm.org/viewvc/llvm-project?rev=206669&view=rev
Log:
Change the ARM assembler to require a :lower16: or :upper16 on non-constant
expressions for mov instructions instead of silently truncating by default.
For the ARM assembler, we want to avoid misleadingly allowing something
like "mov r0, <symbol>" especially when we turn it into a movw and the
expression <symbol> does not have a :lower16: or :upper16" as part of the
expression. We don't want the behavior of silently truncating, which can be
unexpected and lead to bugs that are difficult to find since this is an easy
mistake to make.
This does change the previous behavior of llvm but actually matches an
older gnu assembler that would not allow this but print less useful errors
of like “invalid constant (0x927c0) after fixup” and “unsupported relocation on
symbol foo”. The error for llvm is "immediate expression for mov requires
:lower16: or :upper16" with correct location information on the operand
as shown in the added test cases.
rdar://12342160
Modified:
llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
llvm/trunk/test/MC/ARM/arm_fixups.s
llvm/trunk/test/MC/ARM/complex-operands.s
llvm/trunk/test/MC/ARM/diagnostics.s
llvm/trunk/test/MC/ARM/thumb2-diagnostics.s
Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Fri Apr 18 18:06:39 2014
@@ -5863,6 +5863,30 @@ validateInstruction(MCInst &Inst,
return Error(Operands[Op]->getStartLoc(), "branch target out of range");
break;
}
+ case ARM::MOVi16:
+ case ARM::t2MOVi16:
+ case ARM::t2MOVTi16:
+ {
+ // We want to avoid misleadingly allowing something like "mov r0, <symbol>"
+ // especially when we turn it into a movw and the expression <symbol> does
+ // not have a :lower16: or :upper16 as part of the expression. We don't
+ // want the behavior of silently truncating, which can be unexpected and
+ // lead to bugs that are difficult to find since this is an easy mistake
+ // to make.
+ int i = (Operands[3]->isImm()) ? 3 : 4;
+ ARMOperand *Op = static_cast<ARMOperand*>(Operands[i]);
+ const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(Op->getImm());
+ if (CE) break;
+ const MCExpr *E = dyn_cast<MCExpr>(Op->getImm());
+ if (!E) break;
+ const ARMMCExpr *ARM16Expr = dyn_cast<ARMMCExpr>(E);
+ if (!ARM16Expr || (ARM16Expr->getKind() != ARMMCExpr::VK_ARM_HI16 &&
+ ARM16Expr->getKind() != ARMMCExpr::VK_ARM_LO16)) {
+ return Error(Op->getStartLoc(),
+ "immediate expression for mov requires :lower16: or :upper16");
+ break;
+ }
+ }
}
return false;
Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp (original)
+++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp Fri Apr 18 18:06:39 2014
@@ -1040,12 +1040,12 @@ ARMMCCodeEmitter::getHiLo16ImmOpValue(co
return 0;
}
// If the expression doesn't have :upper16: or :lower16: on it,
- // it's just a plain immediate expression, and those evaluate to
+ // it's just a plain immediate expression, previously those evaluated to
// the lower 16 bits of the expression regardless of whether
- // we have a movt or a movw.
- Kind = MCFixupKind(isThumb2(STI) ? ARM::fixup_t2_movw_lo16
- : ARM::fixup_arm_movw_lo16);
- Fixups.push_back(MCFixup::Create(0, E, Kind, MI.getLoc()));
+ // we have a movt or a movw, but that led to misleadingly results.
+ // This is now disallowed in the the AsmParser in validateInstruction()
+ // so this should never happen.
+ assert(0 && "expression without :upper16: or :lower16:");
return 0;
}
Modified: llvm/trunk/test/MC/ARM/arm_fixups.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/arm_fixups.s?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/test/MC/ARM/arm_fixups.s (original)
+++ llvm/trunk/test/MC/ARM/arm_fixups.s Fri Apr 18 18:06:39 2014
@@ -26,9 +26,9 @@
@ CHECK-BE: movt r9, :upper16:_foo @ encoding: [0xe3,0b0100AAAA,0x90'A',A]
@ CHECK-BE: @ fixup A - offset: 0, value: _foo, kind: fixup_arm_movt_hi16
- mov r2, fred
+ mov r2, :lower16:fred
-@ CHECK: movw r2, fred @ encoding: [A,0x20'A',0b0000AAAA,0xe3]
+@ CHECK: movw r2, :lower16:fred @ encoding: [A,0x20'A',0b0000AAAA,0xe3]
@ CHECK: @ fixup A - offset: 0, value: fred, kind: fixup_arm_movw_lo16
-@ CHECK-BE: movw r2, fred @ encoding: [0xe3,0b0000AAAA,0x20'A',A]
+@ CHECK-BE: movw r2, :lower16:fred @ encoding: [0xe3,0b0000AAAA,0x20'A',A]
@ CHECK-BE: @ fixup A - offset: 0, value: fred, kind: fixup_arm_movw_lo16
Modified: llvm/trunk/test/MC/ARM/complex-operands.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/complex-operands.s?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/test/MC/ARM/complex-operands.s (original)
+++ llvm/trunk/test/MC/ARM/complex-operands.s Fri Apr 18 18:06:39 2014
@@ -21,20 +21,20 @@ return:
.global arm_function
.type arm_function,%function
arm_function:
- mov r0, #(.L_table_end - .L_table_begin) >> 2
+ mov r0, #:lower16:((.L_table_end - .L_table_begin) >> 2)
blx return
@ CHECK-LABEL: arm_function
-@ CHECK: movw r0, #(.L_table_end-.L_table_begin)>>2
+@ CHECK: movw r0, :lower16:((.L_table_end-.L_table_begin)>>2)
@ CHECK: blx return
.global thumb_function
.type thumb_function,%function
thumb_function:
- mov r0, #(.L_table_end - .L_table_begin) >> 2
+ mov r0, #:lower16:((.L_table_end - .L_table_begin) >> 2)
blx return
@ CHECK-LABEL: thumb_function
-@ CHECK: movw r0, #(.L_table_end-.L_table_begin)>>2
+@ CHECK: movw r0, :lower16:((.L_table_end-.L_table_begin)>>2)
@ CHECK: blx return
Modified: llvm/trunk/test/MC/ARM/diagnostics.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/diagnostics.s?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/test/MC/ARM/diagnostics.s (original)
+++ llvm/trunk/test/MC/ARM/diagnostics.s Fri Apr 18 18:06:39 2014
@@ -465,3 +465,11 @@
ldm sp!, {r0}^
@ CHECK-ERRORS: error: system STM cannot have writeback register
@ CHECK-ERRORS: error: writeback register only allowed on system LDM if PC in register-list
+
+foo2:
+ mov r0, foo2
+ movw r0, foo2
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS: ^
Modified: llvm/trunk/test/MC/ARM/thumb2-diagnostics.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/thumb2-diagnostics.s?rev=206669&r1=206668&r2=206669&view=diff
==============================================================================
--- llvm/trunk/test/MC/ARM/thumb2-diagnostics.s (original)
+++ llvm/trunk/test/MC/ARM/thumb2-diagnostics.s Fri Apr 18 18:06:39 2014
@@ -70,3 +70,14 @@
@ CHECK-ERRORS: error: branch target out of range
@ CHECK-ERRORS: error: branch target out of range
@ CHECK-ERRORS: error: branch target out of range
+
+foo2:
+ mov r0, foo2
+ movw r0, foo2
+ movt r0, foo2
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS: ^
More information about the llvm-commits
mailing list