This is an archive of the discontinued LLVM Phabricator instance.

Remove GlobalValue::getAlignment().
ClosedPublic

Authored by efriedma on May 21 2020, 1:25 AM.

Details

Summary

This function is deceptive at best; it's basically impossible to handle the return value correctly. If you have an arbitrary GlobalValue and you want to determine the alignment of that pointer, Value::getPointerAlignment() returns the correct value. If you want the actual declared alignment of a function or variable, GlobalObject::getAlignment() returns that.

This patch switches all the users of GlobalValue::getAlignment() to an appropriate alternative.

There are target-specific changes to AArch64, AMDGPU, PowerPC, SystemZ, and XCore; the changes seem reasonable to me, but probably could use a second look.

Diff Detail

Event Timeline

efriedma created this revision.May 21 2020, 1:25 AM
arsenm added inline comments.May 21 2020, 7:39 AM
llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
1319–4587

These look fine

nigelp-xmos added inline comments.
llvm/lib/Target/XCore/XCoreISelLowering.cpp
438

Looks good on XCore. (I am not an approver. I have only reviewed for XCore.)

Minor note. I think if we don't get a complaint this is OK.

llvm/lib/IR/Globals.cpp
119

Yeah, this seems wrong.

llvm/lib/LTO/LTOModule.cpp
416–417

Isn't 1 the right choice here?

efriedma marked an inline comment as done.May 28 2020, 11:34 AM
efriedma added inline comments.
llvm/lib/LTO/LTOModule.cpp
416–417

I don't think the difference between 1 and 0 has any practical effect, the way the code is currently written. I guess I can clean this up to use Align, though.

efriedma updated this revision to Diff 267988.Jun 2 2020, 1:59 PM

Address review comment

jdoerfert accepted this revision.Jun 2 2020, 2:20 PM

LGTM.

This revision is now accepted and ready to land.Jun 2 2020, 2:20 PM
This revision was automatically updated to reflect the committed changes.