[PATCH] D77454: [WIP] LoadInst should store Align, not MaybeAlign.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 3 23:14:19 PDT 2020


efriedma created this revision.
efriedma added reviewers: gchatelet, courbet.
Herald added subscribers: asbirlea, jfb, hiraditya, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a project: LLVM.

The fact that loads and stores can have the alignment missing is a constant source of confusion: code that usually works can break down in rare cases.  So fix the LoadInst API so the alignment is never missing.

To reduce the number of changes required to make this work, IRBuilder and certain LoadInst constructors will grab the module's datalayout and compute the alignment automatically.  This is the same alignment instcombine would eventually apply anyway; we're just doing it earlier. There's a minor risk that the way we're retrieving the datalayout could break out-of-tree code, but I don't think that's likely.

I tried to write the changes here in a way that allows applying each bit independently.  So we don't have to apply the whole patch at once; we can review and apply chunks independently. This is taking advantage of two tricks:

1. We can pass the result of getAlign() to setAlignment(), with or without the changes to LoadInst.
2. There's an implicit conversion from Align to MaybeAlign, so various places that expect a MaybeAlign can also take an Align.

This doesn't include test changes. A bunch of regression tests break because the CHECK lines aren't expecting the alignment to get printed. And some codegen regression tests produce different assembly because the backend didn't handle unspecified alignment consistently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77454

Files:
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
  llvm/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
  llvm/lib/Transforms/Scalar/GVN.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/lib/Transforms/Scalar/JumpThreading.cpp
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/VNCoercion.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  polly/lib/CodeGen/IslNodeBuilder.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77454.255005.patch
Type: text/x-patch
Size: 37309 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200404/59d93faf/attachment.bin>


More information about the llvm-commits mailing list