Page MenuHomePhabricator

[AVR] Fix a bug of register allocation
AbandonedPublic

Authored by benshi001 on Dec 30 2021, 3:58 AM.

Diff Detail

Unit TestsFailed

TimeTest
70 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm

Event Timeline

benshi001 created this revision.Dec 30 2021, 3:58 AM
benshi001 requested review of this revision.Dec 30 2021, 3:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 30 2021, 3:58 AM
benshi001 added inline comments.Dec 30 2021, 4:04 AM
llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
171

the previous variable AM is not longer used, so merge it to the if condition.

378

The deleted operand RegZ triggered the ran out of register error, actually we need not spiecify any implicit operands when calling getMachineNode.

393

We already have ReplaceUses(SDValue(N, 1), SDValue(ResNode, 1)); in the end of this function, any same lines in other places of current function AVRDAGToDAGISel::select<ISD::LOAD>(SDNode *N) are deleted.

llvm/test/CodeGen/AVR/lpmx.ll
2

-O1 / -O2 / -O3 are normal, and only -O0 triggers the error.

benshi001 added inline comments.Dec 30 2021, 4:05 AM
llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
185

This form is better for future patches which will add ELPM.

Can you explain what the bug is and why this patch fixes it? I find it hard to interpret the diff.

benshi001 added a comment.EditedJan 4 2022, 5:13 AM

Can you explain what the bug is and why this patch fixes it? I find it hard to interpret the diff.

Sorry, I should make some comments on my changes.

The key reason leaded to the crash was the line in previous code

CurDAG->getMachineNode(LPMOpc, DL, VT, MVT::i16, MVT::Other, Ptr, RegZ);

in which the implicit operand RegZ was explicitly specified in call to getMachineNode, and I remove the RegZ and change the above line to

CurDAG->getMachineNode(LPMOpc, DL, VT, MVT::i16, MVT::Other, Ptr);

then the crash disappears.

Actually there are four getMachineNode get RegZ removed. and others changes are NFC (just simplification for my future patches.)

Can you explain what the bug is and why this patch fixes it? I find it hard to interpret the diff.

The bug is recorded at https://github.com/llvm/llvm-project/issues/52839

There is a crash caused by the implicit operand RegZ is explicitly specified when calling CurDAG->getMachineNode.

benshi001 added inline comments.Jan 10 2022, 12:06 AM
llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
171

All changes in this function selectIndexedProgMemLoad are non functional changes, and they just make the code more clear.

184

otherwise (Offs != 2) Opcode is still 0, which means no match.