Page MenuHomePhabricator

[Alignment][NFC] migrate DataLayout internal struct to llvm::Align
Needs ReviewPublic

Authored by gchatelet on Tue, Sep 10, 7:46 AM.

Details

Reviewers
courbet
majnemer
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

With this patch the sizeof(PointerAlignElem) struct goes from 20B to 16B, while sizeof(LayoutAlignElem) stays at 8B (the struct is padded)

Event Timeline

gchatelet created this revision.Tue, Sep 10, 7:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Sep 10, 7:46 AM
gchatelet edited the summary of this revision. (Show Details)Tue, Sep 10, 7:52 AM
courbet added inline comments.Tue, Sep 10, 8:00 AM
llvm/lib/IR/DataLayout.cpp
54

const

369–376

I would move this back to DataLayout::setAlignment, or ask for a review to the original authors. They might have had other reasons beyond storage for guarding against this size.

gchatelet marked 2 inline comments as done.Tue, Sep 10, 8:07 AM
gchatelet added inline comments.
llvm/lib/IR/DataLayout.cpp
369–376

@majnemer it looks like the checks are only because of implementation details (from 1b9fc3a186aed6ec120688e4d957466f72db3ed5)
Would it make sense to remove them as well as the tests?

gchatelet updated this revision to Diff 219547.Tue, Sep 10, 8:07 AM
  • Address comments
gchatelet marked an inline comment as not done.Tue, Sep 10, 8:08 AM