Page MenuHomePhabricator

[MIRPrinter] Fix incorrect output of unnamed stack names
ClosedPublic

Authored by ehjogab on Dec 22 2020, 3:29 AM.

Details

Summary

The MIRParser expects unnamed stack entries to have empty names ('').
In case of unnamed alloca instructions, the MIRPrinter would output
'<unnamed alloca>', which caused the MIRParser to reject the generated
code.

Diff Detail

Event Timeline

ehjogab created this revision.Dec 22 2020, 3:29 AM
ehjogab requested review of this revision.Dec 22 2020, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2020, 3:29 AM

The test isn't Generic since it uses aarch64, it should go under AArch64

llvm/test/CodeGen/MIR/Generic/unnamed-stack.ll
1 ↗(On Diff #313282)

I didn't know && worked in run lines. Should either use a separate run line with the temp file, or just pipe directly to the second llc

5–6 ↗(On Diff #313282)

Probably should check the frame object list too

It seems patch should include update for alloca-crspill.ll test (failed Unit Tests precheckin).

ehjogab updated this revision to Diff 313497.Dec 23 2020, 12:22 AM

Moved testcase to correct location
Split RUN into multiple lines
Added check of frame object list
Fixed failing testcase

ehjogab marked 2 inline comments as done.Dec 23 2020, 12:23 AM
dfukalov added inline comments.Dec 23 2020, 3:38 AM
llvm/test/CodeGen/MIR/AArch64/unnamed-stack.ll
2–3

As I understand, you don't need the second run line, you can just use the output of the first one with "| FileCheck" and without storing a temporary file.

arsenm accepted this revision.Dec 23 2020, 7:16 AM
arsenm added inline comments.
llvm/test/CodeGen/MIR/AArch64/unnamed-stack.ll
4

Add a brief comment explaining that this checks round tripping anonymous allocas

This revision is now accepted and ready to land.Dec 23 2020, 7:16 AM
dfukalov added inline comments.Dec 23 2020, 4:53 PM
llvm/test/CodeGen/MIR/AArch64/unnamed-stack.ll
4

Ok, for roundtrip it can be used like llc -O0 -march aarch64 -global-isel -stop-after=irtranslator -o - %s | llc -x mir llc -march aarch64 -run-pass=none -o - | FileCheck %s without creating a temporary file.

ehjogab updated this revision to Diff 313855.Dec 28 2020, 6:53 AM

Removed use of temporary file.

This revision was landed with ongoing or failed builds.Dec 28 2020, 9:02 AM
This revision was automatically updated to reflect the committed changes.

I submitted this for @ehjogab