This is an archive of the discontinued LLVM Phabricator instance.

[Alignment] Change implementation of TargetCallingConv::OrigAlign
AbandonedPublic

Authored by gchatelet on Oct 21 2019, 4:38 AM.

Details

Summary

This patch makes sure that getOrigAlign() cannot return 0.
It now stores the OrigAlign value as a Log2 instead of Log2 + 1 (see Alignment.h encode and decodeMaybeAlign).
This change is not NFC but all the tests passes.

While reviewing the references to TargetCallingConv::getOrigAlign I stumbled upon this call which seems dubious:

It looks like getOrigAlign (which is an alignment) is stored in CCValAssign::Loc (which is an offset or a Reg).
I traced it back to e95c5b3236cf @t.p.northover do you mind having a look?

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

Event Timeline

gchatelet created this revision.Oct 21 2019, 4:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2019, 4:38 AM

It is possible that the current implementation reserves the zero value as an error, although it is currently not checked.
Maybe it is illegal to call getOrigAlign before setOrigAlign?

If this is the intended behavior then some code is broken, adding an assert(A && "OrigAlign must be set"); in getOrigAlignmakes the following tests fail:

LLVM :: CodeGen/ARM/2013-05-13-DAGCombiner-undef-mask.ll
LLVM :: CodeGen/ARM/GlobalISel/arm-unsupported.ll
LLVM :: CodeGen/ARM/arm-vld1.ll
LLVM :: CodeGen/ARM/arm-vlddup.ll
LLVM :: CodeGen/ARM/fp16-instructions.ll
LLVM :: CodeGen/ARM/fp16-no-condition.ll
LLVM :: CodeGen/ARM/fp16-vminmaxnm-safe.ll
LLVM :: CodeGen/ARM/fp16-vminmaxnm.ll
LLVM :: CodeGen/ARM/inc-of-add.ll
LLVM :: CodeGen/ARM/sub-of-not.ll
LLVM :: CodeGen/ARM/swift-return.ll
LLVM :: CodeGen/ARM/umulo-128-legalisation-lowering.ll
LLVM :: CodeGen/ARM/vld-vst-upgrade.ll
LLVM :: CodeGen/ARM/vldm-sched-a9.ll
LLVM :: CodeGen/ARM/vtrn.ll
LLVM :: CodeGen/ARM/vuzp.ll
LLVM :: CodeGen/ARM/vzip.ll
LLVM :: CodeGen/Mips/cconv/pr33883.ll
LLVM :: CodeGen/Mips/cconv/return-struct.ll
LLVM :: CodeGen/Mips/cconv/vector.ll
LLVM :: CodeGen/Mips/fp16-promote.ll
LLVM :: CodeGen/Mips/return-vector.ll
LLVM :: CodeGen/Thumb/umulo-128-legalisation-lowering.ll
LLVM :: CodeGen/Thumb2/mve-shuffle.ll
LLVM :: CodeGen/Thumb2/umulo-128-legalisation-lowering.ll
gchatelet edited the summary of this revision. (Show Details)Oct 21 2019, 5:09 AM
gchatelet edited the summary of this revision. (Show Details)
gchatelet abandoned this revision.Jun 22 2020, 8:29 AM

This will be implemented in D82307