This is an archive of the discontinued LLVM Phabricator instance.

Differential D41855

[ARM] Fix erroneous availability of SMMLS for Armv7-M
ClosedPublic

Authored by avieira on Jan 9 2018, 2:27 AM.

Details

Reviewers
rengolin
t.p.northover
samparker
Commits
rG26b9de9ebbd3: [ARM] Fix erroneous availability of SMMLS for Armv7-M
rL322360: [ARM] Fix erroneous availability of SMMLS for Armv7-M
Summary

Currently Armv7-M targets will generate a SMMLS instruction which is only available for Armv7E-M which does have DSP extensions in contrary to Armv7-M.

This patch fixes that and adds a test.

Diff Detail

Event Timeline

avieira created this revision.Jan 9 2018, 2:27 AM
Herald added subscribers: kristof.beyls, javed.absar, aemerson. · View Herald TranscriptJan 9 2018, 2:27 AM
samparker added a subscriber: samparker.Jan 9 2018, 3:24 AM
samparker added inline comments.
test/CodeGen/ARM/smml.ll
2

This duplicates line 6, and why remove the v6 triple? And what about a test that ensures v7E-M works?

avieira added inline comments.Jan 9 2018, 3:40 AM
test/CodeGen/ARM/smml.ll
2

It does indeed, didnt see it. As for v7E-M... that was in the other patch, Ill add one here too. Thanks.

avieira updated this revision to Diff 129084.Jan 9 2018, 7:30 AM
avieira added inline comments.
test/CodeGen/ARM/smml.ll
2

Right, I was missing my coffee ... line 2 and 6 are for arm and thumb mode respectively. I had another look at the ARM ARM and for arm mode, v6 accepts SMMLS, for thumb only v6T2 accepts it.

I see in ARM.td that FeatureDSP was only added to armv6t2. I have now added it to Armv6.

samparker accepted this revision.Jan 9 2018, 7:45 AM

LGTM, thanks.

This revision is now accepted and ready to land.Jan 9 2018, 7:45 AM
rogfer01 added a subscriber: rogfer01.Jan 10 2018, 12:55 AM
Closed by commit rL322360: [ARM] Fix erroneous availability of SMMLS for Armv7-M (authored by avieira). · Explain WhyJan 12 2018, 1:22 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
Target/
ARM/
3 lines
2 lines
test/
CodeGen/
ARM/
2 lines

Diff 129084

lib/Target/ARM/ARM.td

Show First 20 Lines • Show All 501 Lines • ▼ Show 20 Lines
def ARMv4t : Architecture<"armv4t", "ARMv4t", [HasV4TOps]>; def ARMv4t : Architecture<"armv4t", "ARMv4t", [HasV4TOps]>;
def ARMv5t : Architecture<"armv5t", "ARMv5t", [HasV5TOps]>; def ARMv5t : Architecture<"armv5t", "ARMv5t", [HasV5TOps]>;
def ARMv5te : Architecture<"armv5te", "ARMv5te", [HasV5TEOps]>; def ARMv5te : Architecture<"armv5te", "ARMv5te", [HasV5TEOps]>;
def ARMv5tej : Architecture<"armv5tej", "ARMv5tej", [HasV5TEOps]>; def ARMv5tej : Architecture<"armv5tej", "ARMv5tej", [HasV5TEOps]>;
def ARMv6 : Architecture<"armv6", "ARMv6", [HasV6Ops]>; def ARMv6 : Architecture<"armv6", "ARMv6", [HasV6Ops,
FeatureDSP]>;
def ARMv6t2 : Architecture<"armv6t2", "ARMv6t2", [HasV6T2Ops, def ARMv6t2 : Architecture<"armv6t2", "ARMv6t2", [HasV6T2Ops,
FeatureDSP]>; FeatureDSP]>;
def ARMv6k : Architecture<"armv6k", "ARMv6k", [HasV6KOps]>; def ARMv6k : Architecture<"armv6k", "ARMv6k", [HasV6KOps]>;
def ARMv6kz : Architecture<"armv6kz", "ARMv6kz", [HasV6KOps, def ARMv6kz : Architecture<"armv6kz", "ARMv6kz", [HasV6KOps,
FeatureTrustZone]>; FeatureTrustZone]>;
▲ Show 20 Lines • Show All 527 Lines • Show Last 20 Lines

lib/Target/ARM/ARMISelDAGToDAG.cpp

Show First 20 Lines • Show All 2,759 Lines • ▼ Show 20 Lines if (Subtarget->isThumb()) {
CurDAG->getRegister(0, MVT::i32) }; CurDAG->getRegister(0, MVT::i32) };
ReplaceNode(N, CurDAG->getMachineNode( ReplaceNode(N, CurDAG->getMachineNode(
Subtarget->hasV6Ops() ? ARM::SMLAL : ARM::SMLALv5, dl, Subtarget->hasV6Ops() ? ARM::SMLAL : ARM::SMLALv5, dl,
MVT::i32, MVT::i32, Ops)); MVT::i32, MVT::i32, Ops));
return; return;
} }
} }
case ARMISD::SUBE: { case ARMISD::SUBE: {
if (!Subtarget->hasV6Ops()) if (!Subtarget->hasV6Ops() || !Subtarget->hasDSP())
break; break;
// Look for a pattern to match SMMLS // Look for a pattern to match SMMLS
// (sube a, (smul_loHi a, b), (subc 0, (smul_LOhi(a, b)))) // (sube a, (smul_loHi a, b), (subc 0, (smul_LOhi(a, b))))
if (N->getOperand(1).getOpcode() != ISD::SMUL_LOHI || if (N->getOperand(1).getOpcode() != ISD::SMUL_LOHI ||
N->getOperand(2).getOpcode() != ARMISD::SUBC || N->getOperand(2).getOpcode() != ARMISD::SUBC ||
!SDValue(N, 1).use_empty()) !SDValue(N, 1).use_empty())
break; break;
▲ Show 20 Lines • Show All 1,332 Lines • Show Last 20 Lines

test/CodeGen/ARM/smml.ll

; RUN: llc -mtriple=arm-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V4 ; RUN: llc -mtriple=arm-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V4
; RUN: llc -mtriple=armv6-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V6 ; RUN: llc -mtriple=armv6-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V6
samparkerUnsubmitted
Not Done
ReplyInline Actions

This duplicates line 6, and why remove the v6 triple? And what about a test that ensures v7E-M works?

samparker: This duplicates line 6, and why remove the v6 triple? And what about a test that ensures v7E-M…
avieiraAuthorUnsubmitted
Not Done
ReplyInline Actions

It does indeed, didnt see it. As for v7E-M... that was in the other patch, Ill add one here too. Thanks.

avieira: It does indeed, didnt see it. As for v7E-M... that was in the other patch, Ill add one here too.
avieiraAuthorUnsubmitted
Not Done
ReplyInline Actions

Right, I was missing my coffee ... line 2 and 6 are for arm and thumb mode respectively. I had another look at the ARM ARM and for arm mode, v6 accepts SMMLS, for thumb only v6T2 accepts it.

I see in ARM.td that FeatureDSP was only added to armv6t2. I have now added it to Armv6.

avieira: Right, I was missing my coffee ... line 2 and 6 are for arm and thumb mode respectively. I had…
; RUN: llc -mtriple=armv7-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V6 ; RUN: llc -mtriple=armv7-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V6
; RUN: llc -mtriple=thumb-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMB ; RUN: llc -mtriple=thumb-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMB
; RUN: llc -mtriple=thumbv6-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6 ; RUN: llc -mtriple=thumbv6-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6
; RUN: llc -mtriple=thumbv6t2-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6T2 ; RUN: llc -mtriple=thumbv6t2-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6T2
; RUN: llc -mtriple=thumbv7-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6T2 ; RUN: llc -mtriple=thumbv7-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6T2
; RUN: llc -mtriple=thumbv7m-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V4
; RUN: llc -mtriple=thumbv7em-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V6T2
define i32 @Test0(i32 %a, i32 %b, i32 %c) nounwind readnone ssp { define i32 @Test0(i32 %a, i32 %b, i32 %c) nounwind readnone ssp {
entry: entry:
; CHECK-LABEL: Test0 ; CHECK-LABEL: Test0
; CHECK-NOT: smmls ; CHECK-NOT: smmls
%conv4 = zext i32 %a to i64 %conv4 = zext i32 %a to i64
%conv1 = sext i32 %b to i64 %conv1 = sext i32 %b to i64
%conv2 = sext i32 %c to i64 %conv2 = sext i32 %c to i64
▲ Show 20 Lines • Show All 57 Lines • Show Last 20 Lines