This is an archive of the discontinued LLVM Phabricator instance.

Support align attribute for return values
ClosedPublic

Authored by apilipenko on Sep 14 2015, 9:10 AM.

Details

Summary

Since D9791 we take alignment into account to decide if it's safe to speculate load. Now pointer has to be both dereferenceable and properly aligned to be speculative loadable. It means that we want to support align attribute everywhere we support dereferenceable attribute.

Align attribute was supported for function arguments only. Support this attribute for return values as well.

Diff Detail

Event Timeline

apilipenko updated this revision to Diff 34684.Sep 14 2015, 9:10 AM
apilipenko retitled this revision from to Support align attribute for return values.
apilipenko updated this object.
apilipenko added reviewers: hfinkel, reames, sanjoy.
apilipenko added a subscriber: llvm-commits.
apilipenko updated this object.Sep 14 2015, 10:54 AM
reames added inline comments.Sep 14 2015, 11:32 AM
lib/AsmParser/LLParser.cpp
1382

The fact your changing the IR parser and not the IR writer, bitcode parser, or bitcode writer seems mildly suspect. Have you tested the ability to round trip through both IR and Bitcode?

test/Analysis/ValueTracking/memory-dereferenceable.ll
120

The syntax here feels really weird for me. "align 16" vs "align(16)" feels very inconsistent with "dereferenceable(32)". Not saying we need to fix that right now, just noting it.

apilipenko added inline comments.Sep 15 2015, 5:08 AM
lib/AsmParser/LLParser.cpp
1382

Yes, I have. Other parts work fine.

apilipenko added inline comments.Sep 15 2015, 5:23 AM
test/Analysis/ValueTracking/memory-dereferenceable.ll
120

BTW, do we care about backward compatibility of textual representation?

reames accepted this revision.Sep 16 2015, 6:38 PM
reames edited edge metadata.

LGTM

test/Analysis/ValueTracking/memory-dereferenceable.ll
120

Not really. I'd say don't break it needlessly, but the IR is a living format with no compatibility guarantees. Provided all the in-tree tests have been updated, that's it. (If you are going to change format, please do so in a separate change.)

This revision is now accepted and ready to land.Sep 16 2015, 6:38 PM
This revision was automatically updated to reflect the committed changes.