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/
2 lines
test/
CodeGen/
ARM/
3 lines

Diff 129046

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=armv6t2-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
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