This is an archive of the discontinued LLVM Phabricator instance.

Differential D75437

[AVR] Fix incorrect register state for LDRdPtr
ClosedPublic

Authored by Jim on Mar 2 2020, 12:50 AM.

Details

Reviewers
dylanmckay
Commits
rG4e3b037665fd: [AVR] Fix incorrect register state for LDRdPtr
Summary

LDRdPtr expanded from LDWRdPtr shouldn't define its second operand(SrcReg).
The second operand is its source register.
Add -verify-machineinstrs into command line of testcases can trigger this error.

Diff Detail

Event Timeline

Jim created this revision.Mar 2 2020, 12:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2020, 12:50 AM
Herald added subscribers: llvm-commits, hiraditya. · View Herald Transcript
Harbormaster failed remote builds in B47747: Diff 247563!Mar 2 2020, 1:31 AM
Jim updated this revision to Diff 247787.Mar 2 2020, 10:21 PM

Add -verify-machineinstrs for brind.ll

dylanmckay added a comment.Mar 2 2020, 10:50 PM

Solid fix, once the lint errors are fixed I will approve.

dylanmckay accepted this revision.Mar 2 2020, 10:51 PM

Actually I'll just approve under condition that the pre-merge clang-format checks are fixed before commit.

This revision is now accepted and ready to land.Mar 2 2020, 10:51 PM
Harbormaster failed remote builds in B47857: Diff 247787!Mar 2 2020, 11:09 PM
Jim updated this revision to Diff 247802.Mar 2 2020, 11:38 PM

Format code.

dylanmckay accepted this revision.Mar 3 2020, 12:15 AM
Harbormaster failed remote builds in B47867: Diff 247802!Mar 3 2020, 12:24 AM
Closed by commit rG4e3b037665fd: [AVR] Fix incorrect register state for LDRdPtr (authored by Jim). · Explain WhyMar 3 2020, 1:43 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Target/
AVR/
2 lines
test/
CodeGen/
AVR/
2 lines
2 lines
pseudo/
2 lines

Diff 247563

llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

Show First 20 Lines • Show All 591 Lines • ▼ Show 20 Linesbool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
if (DstReg == SrcReg) if (DstReg == SrcReg)
TmpReg = scavengeGPR8(MI); TmpReg = scavengeGPR8(MI);
unsigned CurDstLoReg = (DstReg == SrcReg) ? TmpReg : DstLoReg; unsigned CurDstLoReg = (DstReg == SrcReg) ? TmpReg : DstLoReg;
unsigned CurDstHiReg = (DstReg == SrcReg) ? TmpReg : DstHiReg; unsigned CurDstHiReg = (DstReg == SrcReg) ? TmpReg : DstHiReg;
// Load low byte. // Load low byte.
auto MIBLO = buildMI(MBB, MBBI, OpLo) auto MIBLO = buildMI(MBB, MBBI, OpLo)
.addReg(CurDstLoReg, RegState::Define) .addReg(CurDstLoReg, RegState::Define)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-    .addReg(CurDstLoReg, RegState::Define)
-    .addReg(SrcReg);
+                   .addReg(CurDstLoReg, RegState::Define)
+                   .addReg(SrcReg);
Lint: Pre-merge checks: clang-format: please reformat the code ``` - .addReg(CurDstLoReg, RegState::Define) - .
.addReg(SrcReg, RegState::Define); .addReg(SrcReg);
// Push low byte onto stack if necessary. // Push low byte onto stack if necessary.
if (TmpReg) if (TmpReg)
buildMI(MBB, MBBI, AVR::PUSHRr).addReg(TmpReg); buildMI(MBB, MBBI, AVR::PUSHRr).addReg(TmpReg);
// Load high byte. // Load high byte.
auto MIBHI = buildMI(MBB, MBBI, OpHi) auto MIBHI = buildMI(MBB, MBBI, OpHi)
.addReg(CurDstHiReg, RegState::Define) .addReg(CurDstHiReg, RegState::Define)
▲ Show 20 Lines • Show All 1,069 Lines • Show Last 20 Lines

llvm/test/CodeGen/AVR/PR37143.ll

; RUN: llc -mattr=avr6,sram < %s -march=avr | FileCheck %s ; RUN: llc -mattr=avr6,sram < %s -march=avr -verify-machineinstrs | FileCheck %s
; CHECK: ld {{r[0-9]+}}, [[PTR:[XYZ]]] ; CHECK: ld {{r[0-9]+}}, [[PTR:[XYZ]]]
; CHECK: ldd {{r[0-9]+}}, [[PTR]]+1 ; CHECK: ldd {{r[0-9]+}}, [[PTR]]+1
; CHECK: st [[PTR2:[XYZ]]], {{r[0-9]+}} ; CHECK: st [[PTR2:[XYZ]]], {{r[0-9]+}}
; CHECK: std [[PTR2]]+1, {{r[0-9]+}} ; CHECK: std [[PTR2]]+1, {{r[0-9]+}}
define void @load_store_16(i16* nocapture %ptr) local_unnamed_addr #1 { define void @load_store_16(i16* nocapture %ptr) local_unnamed_addr #1 {
entry: entry:
%0 = load i16, i16* %ptr, align 2 %0 = load i16, i16* %ptr, align 2
%add = add i16 %0, 5 %add = add i16 %0, 5
store i16 %add, i16* %ptr, align 2 store i16 %add, i16* %ptr, align 2
ret void ret void
} }

llvm/test/CodeGen/AVR/load.ll

; RUN: llc -mattr=avr6,sram < %s -march=avr | FileCheck %s ; RUN: llc -mattr=avr6,sram < %s -march=avr -verify-machineinstrs | FileCheck %s
define i8 @load8(i8* %x) { define i8 @load8(i8* %x) {
; CHECK-LABEL: load8: ; CHECK-LABEL: load8:
; CHECK: ld r24, {{[XYZ]}} ; CHECK: ld r24, {{[XYZ]}}
%1 = load i8, i8* %x %1 = load i8, i8* %x
ret i8 %1 ret i8 %1
} }
▲ Show 20 Lines • Show All 133 Lines • Show Last 20 Lines

llvm/test/CodeGen/AVR/pseudo/LDWRdPtr.mir

Show All 11 Lines
--- ---
name: test_ldwrdptr name: test_ldwrdptr
body: | body: |
bb.0.entry: bb.0.entry:
; CHECK-LABEL: test_ldwrdptr ; CHECK-LABEL: test_ldwrdptr
; CHECK: $r0, $r31r30 = LDRdPtr ; CHECK: $r0 = LDRdPtr $r31r30
; CHECK-NEXT: $r1 = LDDRdPtrQ $r31r30, 1 ; CHECK-NEXT: $r1 = LDDRdPtrQ $r31r30, 1
$r1r0 = LDWRdPtr $r31r30 $r1r0 = LDWRdPtr $r31r30
... ...