This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][IRTranslator] Fix crash during translation of zero sized loads and stores
ClosedPublic

Authored by aemerson on Nov 29 2017, 7:49 AM.

Details

Summary

[GlobalISel][IRTranslator] Fix crash during translation of zero sized loads/stores/args/returns.

This fixes PR35358.

Diff Detail

Repository
rL LLVM

Event Timeline

aemerson created this revision.Nov 29 2017, 7:49 AM
dsanders added inline comments.Nov 29 2017, 8:38 AM
lib/CodeGen/GlobalISel/IRTranslator.cpp
340–341 ↗(On Diff #124750)

I'm not sure this is correct for the load. If the load result has a use then I'd expect -verify-machineinstrs to complain about a missing def. I think we need to emit an IMPLICIT_DEF.

aemerson added inline comments.Nov 29 2017, 9:27 AM
lib/CodeGen/GlobalISel/IRTranslator.cpp
340–341 ↗(On Diff #124750)

That would require getting an LLT for a zero size type to create a vreg and running into the initial cause of the crash.

Also, is there anything meaningful you can do with the load result? You can't bitcast it to anything (I think?) and storing it won't result in a store being generated, so no instructions are emitted for the verifier.

dsanders added inline comments.Nov 29 2017, 9:41 AM
lib/CodeGen/GlobalISel/IRTranslator.cpp
340–341 ↗(On Diff #124750)

You can return them and pass them to functions but that's the only cases I can think of. If we eliminate those uses too then it will LGTM

aemerson added inline comments.Nov 29 2017, 10:08 AM
lib/CodeGen/GlobalISel/IRTranslator.cpp
340–341 ↗(On Diff #124750)

Yep, returns don't handle this properly either, so I'll fix those too.

aemerson updated this revision to Diff 124929.Nov 30 2017, 6:23 AM
aemerson edited the summary of this revision. (Show Details)

Updated patch with additional checking for zero sized arguments and returns.

This revision is now accepted and ready to land.Nov 30 2017, 9:54 AM
This revision was automatically updated to reflect the committed changes.