Page MenuHomePhabricator

[Alignment] Get DataLayout::StackAlignment as Align
ClosedPublic

Authored by gchatelet on Sep 20 2019, 11:24 AM.

Details

Summary

Internally it is needed to know if StackAlignment is set but we can
expose it as llvm::Align.

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

Repository
rL LLVM

Event Timeline

gchatelet created this revision.Sep 20 2019, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2019, 11:24 AM

Can you please explain why the switch from MaybeAlign to Align is fine ? ARMTargetLowering::getABIAlignmentForCallingConv can switch from non-zero to 0 it seems.

I see another call in the WebAsm backend.

Can you please explain why the switch from MaybeAlign to Align is fine ? ARMTargetLowering::getABIAlignmentForCallingConv can switch from non-zero to 0 it seems.

I see another call in the WebAsm backend.

TL;DR: call sites use std::min between two alignments which wouldn't make sense in case one of the two alignments is 0 or undefined.

Careful inspection of all call sites (including the one in WebAssembly) expect an alignment of at least 1.
Tests are still passing with this patch.

courbet accepted this revision.Sep 23 2019, 2:54 AM
This revision is now accepted and ready to land.Sep 23 2019, 2:54 AM
This revision was automatically updated to reflect the committed changes.