Page MenuHomePhabricator

[Alignment][NFC] Attributes use Align/MaybeAlign
ClosedPublic

Authored by gchatelet on Mon, Oct 21, 12:58 PM.

Details

Summary

This is patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

Diff Detail

Event Timeline

gchatelet created this revision.Mon, Oct 21, 12:58 PM
courbet added inline comments.Tue, Oct 22, 12:18 AM
llvm/include/llvm/IR/Attributes.h
803

Please update the comment.

805–813

is not what ? :)

815

ditto

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
1573–1574

align < 4: do we want this to compile ?

gchatelet updated this revision to Diff 226010.Tue, Oct 22, 2:41 AM
gchatelet marked 5 inline comments as done.
  • Address comments
llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
1573–1574

We need to be able to compare an alignment with an offset for instance, in which case there is no power of two guarantee.
In that sense, we want this to compile.

Now in this case I agree that it should be align < Align(4)

courbet accepted this revision.Tue, Oct 22, 2:47 AM
This revision is now accepted and ready to land.Tue, Oct 22, 2:47 AM
This revision was automatically updated to reflect the committed changes.