This is an archive of the discontinued LLVM Phabricator instance.

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
116	I think you could simplify this pattern a bit like this: def t2_so_imm_neg : Operand<i32>, ImmLeaf<i32, [{ return Imm && ARM_AM::getT2SOImmVal(-(uint32_t)Imm) != -1; }] ...

Diffusion mentioned this in rL298559: [ARM] simplifying t2_so_imm_neg as suggested by Eli Friedman in D31242 (NFC).Mar 22 2017, 4:25 PM

tyomitch marked an inline comment as done.Mar 22 2017, 4:41 PM

vitalybuka removed a reviewer: vitalybuka.Mar 22 2017, 4:41 PM

vitalybuka added a subscriber: vitalybuka.

Diffusion mentioned this in rL298562: Reapply r298417 "[ARM] Recommit the glueless lowering of addc/adde in Thumb1".Mar 22 2017, 4:48 PM

Revision Contents

Path

Size

llvm/

trunk/

lib/

Target/

ARM/

ARMInstrThumb2.td

4 lines

test/

CodeGen/

Thumb/

ispositive.ll

9 lines

Diff 92641

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

Show First 20 Lines • Show All 105 Lines • ▼ Show 20 Lines	def t2_so_imm_notSext : Operand<i32>, PatLeaf<(imm), [{
return ARM_AM::getT2SOImmVal(~N16bitSignExt) != -1;		return ARM_AM::getT2SOImmVal(~N16bitSignExt) != -1;
}], t2_so_imm_notSext16_XFORM> {		}], t2_so_imm_notSext16_XFORM> {
let ParserMatchClass = t2_so_imm_not_asmoperand;		let ParserMatchClass = t2_so_imm_not_asmoperand;
}		}

// t2_so_imm_neg - Match an immediate that is a negation of a t2_so_imm.		// t2_so_imm_neg - Match an immediate that is a negation of a t2_so_imm.
def t2_so_imm_neg_asmoperand : AsmOperandClass { let Name = "T2SOImmNeg"; }		def t2_so_imm_neg_asmoperand : AsmOperandClass { let Name = "T2SOImmNeg"; }
def t2_so_imm_neg : Operand<i32>, PatLeaf<(imm), [{		def t2_so_imm_neg : Operand<i32>, PatLeaf<(imm), [{
int64_t Value = -(int)N->getZExtValue();		int64_t Value = N->getZExtValue();
		if (Value == 1LL<<31) return false; // INT_MIN cannot be negated
		Value = -(int)Value;
		efriedmaUnsubmitted Done Reply Inline Actions I think you could simplify this pattern a bit like this: def t2_so_imm_neg : Operand<i32>, ImmLeaf<i32, [{ return Imm && ARM_AM::getT2SOImmVal(-(uint32_t)Imm) != -1; }] ... efriedma: I think you could simplify this pattern a bit like this: ``` def t2_so_imm_neg : Operand<i32>…
return Value && ARM_AM::getT2SOImmVal(Value) != -1;		return Value && ARM_AM::getT2SOImmVal(Value) != -1;
}], t2_so_imm_neg_XFORM> {		}], t2_so_imm_neg_XFORM> {
let ParserMatchClass = t2_so_imm_neg_asmoperand;		let ParserMatchClass = t2_so_imm_neg_asmoperand;
}		}

/// imm0_4095 predicate - True if the 32-bit immediate is in the range [0.4095].		/// imm0_4095 predicate - True if the 32-bit immediate is in the range [0.4095].
def imm0_4095_asmoperand: ImmAsmOperand { let Name = "Imm0_4095"; }		def imm0_4095_asmoperand: ImmAsmOperand { let Name = "Imm0_4095"; }
def imm0_4095 : Operand<i32>, ImmLeaf<i32, [{		def imm0_4095 : Operand<i32>, ImmLeaf<i32, [{
▲ Show 20 Lines • Show All 4,616 Lines • Show Last 20 Lines

llvm/trunk/test/CodeGen/Thumb/ispositive.ll

	; RUN: llc -mtriple=thumb-eabi %s -o - \| FileCheck %s			; RUN: llc -mtriple=thumb-eabi %s -o - \| FileCheck %s

	define i32 @test1(i32 %X) {			define i32 @test1(i32 %X) {
	entry:			entry:
	; CHECK-LABEL: test1:			; CHECK-LABEL: test1:
	; CHECK: lsrs r0, r0, #31			; CHECK: lsrs r0, r0, #31
	icmp slt i32 %X, 0 ; <i1>:0 [#uses=1]			icmp slt i32 %X, 0 ; <i1>:0 [#uses=1]
	zext i1 %0 to i32 ; <i32>:1 [#uses=1]			zext i1 %0 to i32 ; <i32>:1 [#uses=1]
	ret i32 %1			ret i32 %1
	}			}

				define i32 @test2(i32 %X) {
				entry:
				; CHECK-LABEL: test2:
				; CHECK: lsls r1, r1, #31
				; CHECK-NEXT: adds
				%tmp1 = sub i32 %X, 2147483648
				ret i32 %tmp1
				}

This is an archive of the discontinued LLVM Phabricator instance.

[ARM] t2_so_imm_neg had a subtle bug in the conversion, and could trigger UB by negating (int)-2147483648. By pure luck, none of the pre-existing tests triggered this; so I'm adding one.ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 92641

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

llvm/trunk/test/CodeGen/Thumb/ispositive.ll

[ARM] t2_so_imm_neg had a subtle bug in the conversion, and could trigger UB by negating (int)-2147483648. By pure luck, none of the pre-existing tests triggered this; so I'm adding one.
ClosedPublic