This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Fix creating MMOs with align 0
ClosedPublic

Authored by arsenm on Jan 23 2019, 9:38 PM.

Diff Detail

Event Timeline

arsenm created this revision.Jan 23 2019, 9:38 PM
aemerson added inline comments.Jan 25 2019, 10:34 AM
lib/CodeGen/GlobalISel/IRTranslator.cpp
875

Can we use DL.getABITypeAlignment() here?

arsenm marked an inline comment as done.Jan 25 2019, 10:49 AM
arsenm added inline comments.
lib/CodeGen/GlobalISel/IRTranslator.cpp
875

Probably? I don't know anything about vastart. I'm not sure why this has an MMO in the first place

aemerson added inline comments.Jan 25 2019, 4:25 PM
lib/CodeGen/GlobalISel/IRTranslator.cpp
875

vastart's argument is a target dependent pointer to a va_list object on the stack, so I think it's worth it to try to get the right alignment. That said, 1 is always safe so I think it's ok to use for now.

Petar.Avramovic added inline comments.
lib/Target/Mips/MipsCallLowering.cpp
157–160

Hi Matt. Could you use
MinAlign(MIRBuilder.getMF().getSubtarget().getFrameLowering()->getStackAlignment(), Offset)
instead of 1 for alignment.

244–245

Same here.

arsenm updated this revision to Diff 183955.Jan 28 2019, 1:49 PM
arsenm marked an inline comment as done.

Set alignment for mips

arsenm marked an inline comment as done.Jan 28 2019, 1:51 PM
arsenm added inline comments.
lib/CodeGen/GlobalISel/IRTranslator.cpp
875

I've looked at what SelectionDAG does for this, and I'm still not clear on what to use. The targets are creating the MMO there, and each one seems to be doing something different. Most of the cases seem to create a MachinePointerInfo and then get*Store functions magically infer an alignment from the underlying IR value. Some targets are hardcoding this. I think this is a separate patch

aemerson accepted this revision.Jan 28 2019, 3:26 PM
aemerson added inline comments.
lib/CodeGen/GlobalISel/IRTranslator.cpp
875

Ok, fine.

This revision is now accepted and ready to land.Jan 28 2019, 3:26 PM
arsenm closed this revision.Jan 30 2019, 5:38 PM

r352712