This is an archive of the discontinued LLVM Phabricator instance.

Fix PR18364: Only emit movw for ARM architectures that support it
ClosedPublic

Authored by dim on Sep 2 2014, 11:21 AM.

Download Raw Diff

Details

Reviewers

rengolin
t.p.northover

Summary

See http://llvm.org/PR18364 for the initial report and discussion.

The fix is to add a HasV6T2 check to the ARMadde GPR:$src, imm0_65535_neg pattern, and add a testcase.

The only additional problem is that test/CodeGen/ARM/carry.ll fails after this change, because that test does not seem to emit a movw anymore. This may well have to do with the default ARM CPU chosen on a certain host system. The solution is probably to add a more explicit architecture in the llc command for this test. Suggestions welcome.

Diff Detail

Event Timeline

dim updated this revision to Diff 13174.Sep 2 2014, 11:21 AM

dim retitled this revision from to Fix PR18364: Only emit movw for ARM architectures that support it.

dim updated this object.

dim edited the test plan for this revision. (Show Details)

dim added reviewers: rengolin, t.p.northover.

Herald added a subscriber: aemerson. · View Herald TranscriptSep 2 2014, 11:21 AM

dim added a subscriber: Unknown Object (MLST).Sep 2 2014, 11:24 AM

I agree you should add a more specific target for the failing test.

cheers,
--renato

test/CodeGen/ARM/pr18364-movw.ll
2	a bit pedantic, but can you add armv6t2? just to clear the boundaries

Per Renato's suggestions:

Use armv6t2-eabi for test/CodeGen/ARM/carry.ll, so it may emit movw
Add explicit armv6t2 test to test/CodeGen/ARM/pr18364-movw.ll

LGTM, thanks!

--renato

This revision is now accepted and ready to land.Sep 2 2014, 12:30 PM

It would be nice if somebody could commit it for me, as I don't have commit privileges. :)

Committed in r216989 and r216990 (I missed the test).

cheers,
--renato

Revision Contents

Path

Size

lib/

Target/

ARM/

ARMInstrInfo.td

3 lines

test/

CodeGen/

ARM/

pr18364-movw.ll

31 lines

Diff 13174

lib/Target/ARM/ARMInstrInfo.td

Show First 20 Lines • Show All 3,420 Lines • ▼ Show 20 Lines	def : ARMPat<(ARMaddc GPR:$src, imm0_65535_neg:$imm),
Requires<[IsARM, HasV6T2]>;		Requires<[IsARM, HasV6T2]>;

// The with-carry-in form matches bitwise not instead of the negation.		// The with-carry-in form matches bitwise not instead of the negation.
// Effectively, the inverse interpretation of the carry flag already accounts		// Effectively, the inverse interpretation of the carry flag already accounts
// for part of the negation.		// for part of the negation.
def : ARMPat<(ARMadde GPR:$src, so_imm_not:$imm, CPSR),		def : ARMPat<(ARMadde GPR:$src, so_imm_not:$imm, CPSR),
(SBCri GPR:$src, so_imm_not:$imm)>;		(SBCri GPR:$src, so_imm_not:$imm)>;
def : ARMPat<(ARMadde GPR:$src, imm0_65535_neg:$imm, CPSR),		def : ARMPat<(ARMadde GPR:$src, imm0_65535_neg:$imm, CPSR),
(SBCrr GPR:$src, (MOVi16 (imm_not_XFORM imm:$imm)))>;		(SBCrr GPR:$src, (MOVi16 (imm_not_XFORM imm:$imm)))>,
		Requires<[IsARM, HasV6T2]>;

// Note: These are implemented in C++ code, because they have to generate		// Note: These are implemented in C++ code, because they have to generate
// ADD/SUBrs instructions, which use a complex pattern that a xform function		// ADD/SUBrs instructions, which use a complex pattern that a xform function
// cannot produce.		// cannot produce.
// (mul X, 2^n+1) -> (add (X << n), X)		// (mul X, 2^n+1) -> (add (X << n), X)
// (mul X, 2^n-1) -> (rsb X, (X << n))		// (mul X, 2^n-1) -> (rsb X, (X << n))

// ARM Arithmetic Instruction		// ARM Arithmetic Instruction
▲ Show 20 Lines • Show All 2,203 Lines • Show Last 20 Lines

test/CodeGen/ARM/pr18364-movw.ll

This file was added.

				; RUN: llc < %s -mtriple=armv5te \| FileCheck %s --check-prefix=V5
				; RUN: llc < %s -mtriple=armv6 \| FileCheck %s --check-prefix=V6
				rengolinUnsubmitted Not Done Reply Inline Actions a bit pedantic, but can you add armv6t2? just to clear the boundaries rengolin: a bit pedantic, but can you add armv6t2? just to clear the boundaries
				; RUN: llc < %s -mtriple=armv7 \| FileCheck %s --check-prefix=V7
				; PR18364

				define i64 @f() #0 {
				entry:
				; V5-NOT: movw
				; V6-NOT: movw
				; V7: movw
				%y = alloca i64, align 8
				%z = alloca i64, align 8
				store i64 1, i64* %y, align 8
				store i64 11579764786944, i64* %z, align 8
				%0 = load i64* %y, align 8
				%1 = load i64* %z, align 8
				%sub = sub i64 %0, %1
				ret i64 %sub
				}

				define i64 @g(i64 %a, i32 %b) #0 {
				entry:
				; V5-NOT: movw
				; V6-NOT: movw
				; V7: movw
				%0 = mul i64 %a, 86400000
				%mul = add i64 %0, -210866803200000
				%conv = sext i32 %b to i64
				%add = add nsw i64 %mul, %conv
				ret i64 %add
				}