This is an archive of the discontinued LLVM Phabricator instance.

[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

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

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
1–2 ↗(On Diff #313497)

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
3 ↗(On Diff #313497)

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
3 ↗(On Diff #313497)

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