[IR] Don't assume all functions are 4 byte aligned

[michaelplatings]

Summary

This is based on previously discussed patches https://reviews.llvm.org/D55115 and https://reviews.llvm.org/D44781

D44781 was abandoned on the understanding that tracking the alignment of function pointers was unlikely to be beneficial.
D55115 was reverted after it emerged that it caused a code size regression because it removed an assumption about the alignment of function pointers.

This patch combines those two:
Add function pointer alignment to DataLayout.
If the function pointer alignment is set, use it. Otherwise, continue to assume that the alignment is 4 as before.

[lebedev.ri]

Please always upload all patches with -U99999.

[michaelplatings]

Diff with -U99999

[efriedma]

Missing update to PPC backend.

I'm not convinced that we should be modifying the datalayout here, as opposed to adding some sort of target hook for the optimizer.

It's a little unfortunate that there isn't any way to represent that the alignment of a function pointer is the same as the alignment of the function, but has some known minimum value.

llvm/docs/LangRef.rst

2007 ↗

(On Diff #183881)

This needs more description.

llvm/lib/IR/ConstantFold.cpp

1083 ↗

(On Diff #183881)

If you fail to find a datalayout, you have to assume the worst case (that the pointer isn't aligned at all).

1087 ↗

(On Diff #183881)

Using "4" as a default is dangerous; on x86, the alignment of the pointer corresponds to the alignment of the function, but might be less than 4 if it isn't explicitly specified.

[olista01]

Why do you think this would be better as a target hook, rather than in the datalayout? The datalayout already includes information about alignments of different types, and most of the target hooks are concerned with what IR will be efficient for the target, not things which might affect correctness.

[efriedma]

I've thought about it a bit more, and maybe specifying it in the datalayout is appropriate: the representation of function pointers is a fundamental property of the target/ABI. I mean, it's not really that important in most cases... if you conservatively treat all function pointers as completely opaque, you'll usually get reasonable optimization, but it's still a fundamental property of a core IR type.

That said, I think the suggested datalayout modification isn't complete. I think we need two possible specifications:

F<bitwidth>: The alignment of function pointers is independent of the alignment of functions, and is at least <bitwidth>. ARM with Thumb uses F8, PPC use F32/F64 for 32/64-bit targets, MIPS with microMIPS uses F8.
f<bitwidth>: The alignment of function pointers is greater than or equal to the explicit alignment specified on the function, and is at least <bitwidth>. Used for other targets; x86 uses f8, aarch64 uses f32, etc.

There's an argument for using a target hook instead: in some cases, we can tell more about a specific function in some cases, so we want target hook anyway. For example, on ARM, if a function uses an attribute to mark it as an ARM-mode function, the alignment is at least 32 bits; there's no way to specify that in the datalayout. And if we have the target hook, very little code would end up querying the datalayout, so there's hardly any point. (IIRC Chandler actually tried to write some code involving aligned function pointers for LLVM at one point, but it failed because we don't have that target hook.)

[michaelplatings]

Hi @efriedma, sorry for the delayed response.
I've added the features you asked for to DataLayout.
It is intended that this patch preserves the existing behaviour if no function pointer alignment is specified.* Therefore I hope you'll agree that it isn't necessary for me to provide code to use the new feature on all platforms.
I did also look into using a target hook but doing so would require modifying a lot of functions to take a TargetTransformInfo argument. Given that we're in agreement that the DataLayout is an appropriate choice, it seemed like the best option.

• The exception to this is the case where the Function object doesn't have a parent - no alignment is assumed whereas previously 4-byte alignment was assumed. Hopefully this case is rare enough that code size won't be significantly impacted.

[michaelplatings]

llvm/lib/IR/ConstantFold.cpp

1087 ↗

(On Diff #183881)

I've put in a comment to say as much. Sadly I'm not in a position to change this behaviour without causing a code size regression.

[efriedma]

Chandler, when you have a chance, can you look at the LangRef changes, since you put some thought into the design?

I think the DataLayout/LangRef changes look correct.

I agree it isn't necessary to fix every target in the initial patch; it would take a long time to get review, and no one person can properly test every target. But I'm afraid we'll forget to fix some target if someone doesn't go through and post followup patches for every target. (It doesn't really even matter if the initial patches are right, as long as there's something for the target maintainers to look at.)

llvm/include/llvm/IR/ConstantFold.h

7 ↗

(On Diff #188184)

Are you just moving this for the unittests? You shouldn't need to; you can just check the result of ConstantExpr::get(Instruction::And, ...

llvm/lib/IR/ConstantFold.cpp

1087 ↗

(On Diff #183881)

From the discussion on D55115, adding the right default to x86 (Fn8) is probably enough to avoid the regression, but I guess we can deal with that as a followup.

Please add a comment like // FIXME: This code should be deleted once existing targets have appropriate defaults

[miyuki]

llvm/lib/IR/Value.cpp

651 ↗

(On Diff #188184)

This comment needs to be updated

[efriedma]

Probably worth sending an email to llvmdev noting that the datalayout is changing before you merge this, so a wider audience can review the IR aspects of the change.

Otherwise LGTM

[efriedma]

Anyone looking at updating the datalayout for other platforms? (If nobody is looking at all, I'll try to find some time next week, I guess.)

cfe/trunk/lib/Basic/Targets/ARM.cpp

void ARMTargetInfo::setABIAAPCS() {
   UseBitFieldTypeAlignment = true;
 
   ZeroLengthBitfieldBoundary = 0;
 
   // Thumb1 add sp, #imm requires the immediate value be multiple of 4,
   // so set preferred for small types to 32.
   if (T.isOSBinFormatMachO()) {
     resetDataLayout(BigEndian
-                        ? "E-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-                        : "e-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+                        ? "E-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+                        : "e-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
   } else if (T.isOSWindows()) {
     assert(!BigEndian && "Windows on ARM does not support big endian");
     resetDataLayout("e"
                     "-m:w"
                     "-p:32:32"
+                    "-Fi8"
                     "-i64:64"
                     "-v128:64:128"
                     "-a:0:32"
                     "-n32"
                     "-S64");
   } else if (T.isOSNaCl()) {
     assert(!BigEndian && "NaCl on ARM does not support big endian");
-    resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128");
+    resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S128");
   } else {
     resetDataLayout(BigEndian
-                        ? "E-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-                        : "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+                        ? "E-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+                        : "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
   }
 
   // FIXME: Enumerated types are variable width in straight AAPCS.
 }
 
 void ARMTargetInfo::setABIAPCS(bool IsAAPCS16) {
   const llvm::Triple &T = getTriple();
 
void ARMTargetInfo::setABIAPCS(bool IsAAPCS16) {
 
   /// gcc forces the alignment to 4 bytes, regardless of the type of the
   /// zero length bitfield.  This corresponds to EMPTY_FIELD_BOUNDARY in
   /// gcc.
   ZeroLengthBitfieldBoundary = 32;
 
   if (T.isOSBinFormatMachO() && IsAAPCS16) {
     assert(!BigEndian && "AAPCS16 does not support big-endian");
-    resetDataLayout("e-m:o-p:32:32-i64:64-a:0:32-n32-S128");
+    resetDataLayout("e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128");
   } else if (T.isOSBinFormatMachO())
     resetDataLayout(
         BigEndian
-            ? "E-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-            : "e-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+            ? "E-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+            : "e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
   else
     resetDataLayout(
         BigEndian
-            ? "E-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-            : "e-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+            ? "E-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+            : "e-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
 
   // FIXME: Override "preferred align" for double and long long.
 }
 
 void ARMTargetInfo::setArchInfo() {
   StringRef ArchName = getTriple().getArchName();
 
   ArchISA = llvm::ARM::parseArchISA(ArchName);

 }
 
 CygwinARMTargetInfo::CygwinARMTargetInfo(const llvm::Triple &Triple,
                                          const TargetOptions &Opts)
     : ARMleTargetInfo(Triple, Opts) {
   this->WCharType = TargetInfo::UnsignedShort;
   TLSSupported = false;
   DoubleAlign = LongLongAlign = 64;
-  resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+  resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
 }
 
 void CygwinARMTargetInfo::getTargetDefines(const LangOptions &Opts,
                                            MacroBuilder &Builder) const {
   ARMleTargetInfo::getTargetDefines(Opts, Builder);
   Builder.defineMacro("_ARM_");
   Builder.defineMacro("__CYGWIN__");
   Builder.defineMacro("__CYGWIN32__");

cfe/trunk/test/CodeGen/armv7k-abi.c

 // RUN: %clang_cc1 -triple thumbv7k-apple-watchos2.0 -target-abi aapcs16 -target-cpu cortex-a7 %s -o - -emit-llvm | FileCheck %s
 
 #include <arm_neon.h>
 
 // Make sure 64 and 128 bit types are naturally aligned by the v7k ABI:
 
-// CHECK: target datalayout = "e-m:o-p:32:32-i64:64-a:0:32-n32-S128"
+// CHECK: target datalayout = "e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128"
 
 typedef struct {
   float arr[4];
 } HFA;
 
 // CHECK: define void @simple_hfa([4 x float] %h.coerce)
 void simple_hfa(HFA h) {}
 

cfe/trunk/test/CodeGen/target-data.c

 // I686-NACL: target datalayout = "e-m:e-p:32:32-i64:64-n8:16:32-S128"
 
 // RUN: %clang_cc1 -triple x86_64-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=X86_64-NACL
 // X86_64-NACL: target datalayout = "e-m:e-p:32:32-i64:64-n8:16:32:64-S128"
 
 // RUN: %clang_cc1 -triple arm-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=ARM-NACL
-// ARM-NACL: target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128"
+// ARM-NACL: target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S128"
 
 // RUN: %clang_cc1 -triple mipsel-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=MIPS-NACL
 // MIPS-NACL: target datalayout = "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
 
 // RUN: %clang_cc1 -triple le32-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=LE32-NACL
 // LE32-NACL: target datalayout = "e-p:32:32-i64:64"

 // R600SIDefault: target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 // RUN: %clang_cc1 -triple arm64-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=AARCH64
 // AARCH64: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 
 // RUN: %clang_cc1 -triple thumb-unknown-gnueabi -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=THUMB
-// THUMB: target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+// THUMB: target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
 
 // RUN: %clang_cc1 -triple arm-unknown-gnueabi -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=ARM
-// ARM: target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+// ARM: target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
 
 // RUN: %clang_cc1 -triple thumb-unknown -o - -emit-llvm -target-abi apcs-gnu \
 // RUN: %s | FileCheck %s -check-prefix=THUMB-GNU
-// THUMB-GNU: target datalayout = "e-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+// THUMB-GNU: target datalayout = "e-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
 
 // RUN: %clang_cc1 -triple arm-unknown -o - -emit-llvm -target-abi apcs-gnu \
 // RUN: %s | FileCheck %s -check-prefix=ARM-GNU
-// ARM-GNU: target datalayout = "e-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+// ARM-GNU: target datalayout = "e-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
 
 // RUN: %clang_cc1 -triple arc-unknown-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=ARC
 // ARC: target datalayout = "e-m:e-p:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-f32:32:32-i64:32-f64:32-a:0:32-n32"
 
 // RUN: %clang_cc1 -triple hexagon-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=HEXAGON
 // HEXAGON: target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"

llvm/trunk/docs/LangRef.rst

 ``f<size>:<abi>:<pref>``
     This specifies the alignment for a floating-point type of a given bit
     ``<size>``. Only values of ``<size>`` that are supported by the target
     will work. 32 (float) and 64 (double) are supported on all targets; 80
     or 128 (different flavors of long double) are also supported on some
     targets.
 ``a:<abi>:<pref>``
     This specifies the alignment for an object of aggregate type.
+``F<type><abi>``
+    This specifies the alignment for function pointers.
+    The options for ``<type>`` are:
+
+    * ``i``: The alignment of function pointers is independent of the alignment
+      of functions, and is a multiple of ``<abi>``.
+    * ``n``: The alignment of function pointers is a multiple of the explicit
+      alignment specified on the function, and is a multiple of ``<abi>``.
 ``m:<mangling>``
     If present, specifies that llvm names are mangled in the output. Symbols
     prefixed with the mangling escape character ``\01`` are passed through
     directly to the assembler without the escape character. The mangling style
     options are
 
     * ``e``: ELF mangling: Private symbols get a ``.L`` prefix.
     * ``m``: Mips mangling: Private symbols get a ``$`` prefix.

llvm/trunk/include/llvm/IR/DataLayout.h

 
 /// A parsed version of the target data layout string in and methods for
 /// querying it.
 ///
 /// The target data layout string is specified *by the target* - a frontend
 /// generating LLVM IR is required to generate the right target data for the
 /// target being codegen'd to.
 class DataLayout {
+public:
+  enum class FunctionPtrAlignType {
+    /// The function pointer alignment is independent of the function alignment.
+    Independent,
+    /// The function pointer alignment is a multiple of the function alignment.
+    MultipleOfFunctionAlign,
+  };
 private:
   /// Defaults to false.
   bool BigEndian;
 
   unsigned AllocaAddrSpace;
   unsigned StackNaturalAlign;
   unsigned ProgramAddrSpace;
 
+  unsigned FunctionPtrAlign;
+  FunctionPtrAlignType TheFunctionPtrAlignType;
+
   enum ManglingModeT {
     MM_None,
     MM_ELF,
     MM_MachO,
     MM_WinCOFF,
     MM_WinCOFFX86,
     MM_Mips
   };
public:
   ~DataLayout(); // Not virtual, do not subclass this class
 
   DataLayout &operator=(const DataLayout &DL) {
     clear();
     StringRepresentation = DL.StringRepresentation;
     BigEndian = DL.isBigEndian();
     AllocaAddrSpace = DL.AllocaAddrSpace;
     StackNaturalAlign = DL.StackNaturalAlign;
+    FunctionPtrAlign = DL.FunctionPtrAlign;
+    TheFunctionPtrAlignType = DL.TheFunctionPtrAlignType;
     ProgramAddrSpace = DL.ProgramAddrSpace;
     ManglingMode = DL.ManglingMode;
     LegalIntWidths = DL.LegalIntWidths;
     Alignments = DL.Alignments;
     Pointers = DL.Pointers;
     NonIntegralAddressSpaces = DL.NonIntegralAddressSpaces;
     return *this;
   }
public:
   /// Returns true if the given alignment exceeds the natural stack alignment.
   bool exceedsNaturalStackAlignment(unsigned Align) const {
     return (StackNaturalAlign != 0) && (Align > StackNaturalAlign);
   }
 
   unsigned getStackAlignment() const { return StackNaturalAlign; }
   unsigned getAllocaAddrSpace() const { return AllocaAddrSpace; }
 
+  /// Returns the alignment of function pointers, which may or may not be
+  /// related to the alignment of functions.
+  /// \see getFunctionPtrAlignType
+  unsigned getFunctionPtrAlign() const { return FunctionPtrAlign; }
+
+  /// Return the type of function pointer alignment.
+  /// \see getFunctionPtrAlign
+  FunctionPtrAlignType getFunctionPtrAlignType() const {
+    return TheFunctionPtrAlignType;
+  }
+
   unsigned getProgramAddressSpace() const { return ProgramAddrSpace; }
 
   bool hasMicrosoftFastStdCallMangling() const {
     return ManglingMode == MM_WinCOFFX86;
   }
 
   /// Returns true if symbols with leading question marks should not receive IR
   /// mangling. True for Windows mangling modes.

llvm/trunk/lib/IR/ConstantFold.cpp

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
 using namespace llvm;
 using namespace llvm::PatternMatch;
 
case Instruction::And:
             return C1;
         }
 
         // If and'ing the address of a global with a constant, fold it.
         if (CE1->getOpcode() == Instruction::PtrToInt &&
             isa<GlobalValue>(CE1->getOperand(0))) {
           GlobalValue *GV = cast<GlobalValue>(CE1->getOperand(0));
 
-          // Functions are at least 4-byte aligned.
-          unsigned GVAlign = GV->getAlignment();
-          if (isa<Function>(GV))
-            GVAlign = std::max(GVAlign, 4U);
+          unsigned GVAlign;
+
+          if (Module *TheModule = GV->getParent()) {
+            GVAlign = GV->getPointerAlignment(TheModule->getDataLayout());
+
+            // If the function alignment is not specified then assume that it
+            // is 4.
+            // This is dangerous; on x86, the alignment of the pointer
+            // corresponds to the alignment of the function, but might be less
+            // than 4 if it isn't explicitly specified.
+            // However, a fix for this behaviour was reverted because it
+            // increased code size (see https://reviews.llvm.org/D55115)
+            // FIXME: This code should be deleted once existing targets have
+            // appropriate defaults
+            if (GVAlign == 0U && isa<Function>(GV))
+              GVAlign = 4U;
+          } else if (isa<Function>(GV)) {
+            // Without a datalayout we have to assume the worst case: that the
+            // function pointer isn't aligned at all.
+            GVAlign = 0U;
+          } else {
+            GVAlign = GV->getAlignment();
+          }
 
           if (GVAlign > 1) {
             unsigned DstWidth = CI2->getType()->getBitWidth();
             unsigned SrcWidth = std::min(DstWidth, Log2_32(GVAlign));
             APInt BitsNotSet(APInt::getLowBitsSet(DstWidth, SrcWidth));
 
             // If checking bits we know are clear, return zero.
             if ((CI2->getValue() & BitsNotSet) == CI2->getValue())

llvm/trunk/lib/IR/DataLayout.cpp

 void DataLayout::reset(StringRef Desc) {
   clear();
 
   LayoutMap = nullptr;
   BigEndian = false;
   AllocaAddrSpace = 0;
   StackNaturalAlign = 0;
   ProgramAddrSpace = 0;
+  FunctionPtrAlign = 0;
+  TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
   ManglingMode = MM_None;
   NonIntegralAddressSpaces.clear();
 
   // Default alignments
   for (const LayoutAlignElem &E : DefaultAlignments) {
     setAlignment((AlignTypeEnum)E.AlignType, E.ABIAlign, E.PrefAlign,
                  E.TypeBitWidth);
   }
case 'n':  // Native integer types.
           break;
         Split = split(Rest, ':');
       }
       break;
     case 'S': { // Stack natural alignment.
       StackNaturalAlign = inBytes(getInt(Tok));
       break;
     }
+    case 'F': {
+      switch (Tok.front()) {
+      case 'i':
+        TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
+        break;
+      case 'n':
+        TheFunctionPtrAlignType = FunctionPtrAlignType::MultipleOfFunctionAlign;
+        break;
+      default:
+        report_fatal_error("Unknown function pointer alignment type in "
+                           "datalayout string");
+      }
+      Tok = Tok.substr(1);
+      FunctionPtrAlign = inBytes(getInt(Tok));
+      break;
+    }
     case 'P': { // Function address space.
       ProgramAddrSpace = getAddrSpace(Tok);
       break;
     }
     case 'A': { // Default stack/alloca address space.
       AllocaAddrSpace = getAddrSpace(Tok);
       break;
     }

 
 void DataLayout::init(const Module *M) { *this = M->getDataLayout(); }
 
 bool DataLayout::operator==(const DataLayout &Other) const {
   bool Ret = BigEndian == Other.BigEndian &&
              AllocaAddrSpace == Other.AllocaAddrSpace &&
              StackNaturalAlign == Other.StackNaturalAlign &&
              ProgramAddrSpace == Other.ProgramAddrSpace &&
+             FunctionPtrAlign == Other.FunctionPtrAlign &&
+             TheFunctionPtrAlignType == Other.TheFunctionPtrAlignType &&
              ManglingMode == Other.ManglingMode &&
              LegalIntWidths == Other.LegalIntWidths &&
              Alignments == Other.Alignments && Pointers == Other.Pointers;
   // Note: getStringRepresentation() might differs, it is not canonicalized
   return Ret;
 }
 
 DataLayout::AlignmentsTy::iterator

llvm/trunk/lib/IR/Value.cpp

uint64_t Value::getPointerDereferenceableBytes(const DataLayout &DL,
   return DerefBytes;
 }
 
 unsigned Value::getPointerAlignment(const DataLayout &DL) const {
   assert(getType()->isPointerTy() && "must be pointer");
 
   unsigned Align = 0;
   if (auto *GO = dyn_cast<GlobalObject>(this)) {
-    // Don't make any assumptions about function pointer alignment. Some
-    // targets use the LSBs to store additional information.
-    if (isa<Function>(GO))
-      return 0;
+    if (isa<Function>(GO)) {
+      switch (DL.getFunctionPtrAlignType()) {
+      case DataLayout::FunctionPtrAlignType::Independent:
+        return DL.getFunctionPtrAlign();
+      case DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign:
+        return std::max(DL.getFunctionPtrAlign(), GO->getAlignment());
+      default:
+        llvm_unreachable("unknown function pointer align type");
+      }
+    }
     Align = GO->getAlignment();
     if (Align == 0) {
       if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
         Type *ObjectType = GVar->getValueType();
         if (ObjectType->isSized()) {
           // If the object is defined in the current Module, we'll be giving
           // it the preferred alignment. Otherwise, we have to assume that it
           // may only have the minimum ABI alignment.

llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp

else
     // Big endian.
     Ret += "E";
 
   Ret += DataLayout::getManglingComponent(TT);
 
   // Pointers are 32 bits and aligned to 32 bits.
   Ret += "-p:32:32";
 
+  // Function pointers are aligned to 8 bits (because the LSB stores the
+  // ARM/Thumb state).
+  Ret += "-Fi8";
+
   // ABIs other than APCS have 64 bit integers with natural alignment.
   if (ABI != ARMBaseTargetMachine::ARM_ABI_APCS)
     Ret += "-i64:64";
 
   // We have 64 bits floats. The APCS ABI requires them to be aligned to 32
   // bits, others to 64 bits. We always try to align to 64 bits.
   if (ABI == ARMBaseTargetMachine::ARM_ABI_APCS)
     Ret += "-f64:32:64";

llvm/trunk/unittests/IR/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Analysis
   AsmParser
   Core
   Support
   Passes
   )
 
 add_llvm_unittest(IRTests
   AsmWriterTest.cpp
   AttributesTest.cpp
   BasicBlockTest.cpp
   CFGBuilder.cpp
   ConstantRangeTest.cpp
   ConstantsTest.cpp
+  DataLayoutTest.cpp
   DebugInfoTest.cpp
   DebugTypeODRUniquingTest.cpp
   DominatorTreeTest.cpp
   DominatorTreeBatchUpdatesTest.cpp
   FunctionTest.cpp
   PassBuilderCallbacksTest.cpp
   IRBuilderTest.cpp
   InstructionsTest.cpp

llvm/trunk/unittests/IR/ConstantsTest.cpp

TEST(ConstantsTest, BitcastToGEP) {
 
   auto *G = new GlobalVariable(*M, S, false,
                                GlobalValue::ExternalLinkage, nullptr);
   auto *PtrTy = PointerType::get(i32, 0);
   auto *C = ConstantExpr::getBitCast(G, PtrTy);
   ASSERT_EQ(cast<ConstantExpr>(C)->getOpcode(), Instruction::BitCast);
 }
 
+bool foldFuncPtrAndConstToNull(LLVMContext &Context, Module *TheModule,
+                               uint64_t AndValue, unsigned FunctionAlign = 0) {
+  Type *VoidType(Type::getVoidTy(Context));
+  FunctionType *FuncType(FunctionType::get(VoidType, false));
+  Function *Func(Function::Create(
+      FuncType, GlobalValue::ExternalLinkage, "", TheModule));
+
+  if (FunctionAlign) Func->setAlignment(FunctionAlign);
+
+  IntegerType *ConstantIntType(Type::getInt32Ty(Context));
+  ConstantInt *TheConstant(ConstantInt::get(ConstantIntType, AndValue));
+
+  Constant *TheConstantExpr(
+      ConstantExpr::getPtrToInt(Func, ConstantIntType));
+
+  return ConstantExpr::get(Instruction::And, TheConstantExpr,
+                           TheConstant)->isNullValue();
+}
+
+TEST(ConstantsTest, FoldFunctionPtrAlignUnknownAnd2) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  // When the DataLayout doesn't specify a function pointer alignment we
+  // assume in this case that it is 4 byte aligned. This is a bug but we can't
+  // fix it directly because it causes a code size regression on X86.
+  // FIXME: This test should be changed once existing targets have
+  // appropriate defaults. See associated FIXME in ConstantFoldBinaryInstruction
+  ASSERT_TRUE(foldFuncPtrAndConstToNull(Context, &TheModule, 2));
+}
+
+TEST(ConstantsTest, DontFoldFunctionPtrAlignUnknownAnd4) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  ASSERT_FALSE(foldFuncPtrAndConstToNull(Context, &TheModule, 4));
+}
+
+TEST(ConstantsTest, FoldFunctionPtrAlign4) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  const char* AlignmentStrings[] = { "Fi32", "Fn32" };
+
+  for (unsigned AndValue = 1; AndValue <= 2; ++AndValue) {
+    for (const char *AlignmentString : AlignmentStrings) {
+      TheModule.setDataLayout(AlignmentString);
+      ASSERT_TRUE(foldFuncPtrAndConstToNull(Context, &TheModule, AndValue));
+    }
+  }
+}
+
+TEST(ConstantsTest, DontFoldFunctionPtrAlign1) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  const char* AlignmentStrings[] = { "Fi8", "Fn8" };
+
+  for (const char* AlignmentString : AlignmentStrings) {
+    TheModule.setDataLayout(AlignmentString);
+    ASSERT_FALSE(foldFuncPtrAndConstToNull(Context, &TheModule, 2));
+  }
+}
+
+TEST(ConstantsTest, FoldFunctionAlign4PtrAlignMultiple) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  TheModule.setDataLayout("Fn8");
+  ASSERT_TRUE(foldFuncPtrAndConstToNull(Context, &TheModule, 2, 4));
+}
+
+TEST(ConstantsTest, DontFoldFunctionAlign4PtrAlignIndependent) {
+  LLVMContext Context;
+  Module TheModule("TestModule", Context);
+  TheModule.setDataLayout("Fi8");
+  ASSERT_FALSE(foldFuncPtrAndConstToNull(Context, &TheModule, 2, 4));
+}
+
+TEST(ConstantsTest, DontFoldFunctionPtrIfNoModule) {
+  LLVMContext Context;
+  // Even though the function is explicitly 4 byte aligned, in the absence of a
+  // DataLayout we can't assume that the function pointer is aligned.
+  ASSERT_FALSE(foldFuncPtrAndConstToNull(Context, nullptr, 2, 4));
+}
+
+TEST(ConstantsTest, FoldGlobalVariablePtr) {
+  LLVMContext Context;
+
+
+  IntegerType *IntType(Type::getInt32Ty(Context));
+
+  GlobalVariable Global(IntType, true, GlobalValue::ExternalLinkage);
+
+  Global.setAlignment(4);
+
+  ConstantInt *TheConstant(ConstantInt::get(IntType, 2));
+
+  Constant *TheConstantExpr(
+      ConstantExpr::getPtrToInt(&Global, IntType));
+
+  ASSERT_TRUE(ConstantExpr::get( \
+      Instruction::And, TheConstantExpr, TheConstant)->isNullValue());
+}
+
 }  // end anonymous namespace
 }  // end namespace llvm

llvm/trunk/unittests/IR/DataLayoutTest.cpp

+//===- ConstantRangeTest.cpp - ConstantRange tests ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/DataLayout.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(DataLayoutTest, FunctionPtrAlign) {
+  EXPECT_EQ(0U, DataLayout("").getFunctionPtrAlign());
+  EXPECT_EQ(1U, DataLayout("Fi8").getFunctionPtrAlign());
+  EXPECT_EQ(2U, DataLayout("Fi16").getFunctionPtrAlign());
+  EXPECT_EQ(4U, DataLayout("Fi32").getFunctionPtrAlign());
+  EXPECT_EQ(8U, DataLayout("Fi64").getFunctionPtrAlign());
+  EXPECT_EQ(1U, DataLayout("Fn8").getFunctionPtrAlign());
+  EXPECT_EQ(2U, DataLayout("Fn16").getFunctionPtrAlign());
+  EXPECT_EQ(4U, DataLayout("Fn32").getFunctionPtrAlign());
+  EXPECT_EQ(8U, DataLayout("Fn64").getFunctionPtrAlign());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent, \
+      DataLayout("").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent, \
+      DataLayout("Fi8").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign, \
+      DataLayout("Fn8").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout("Fi8"), DataLayout("Fi8"));
+  EXPECT_NE(DataLayout("Fi8"), DataLayout("Fi16"));
+  EXPECT_NE(DataLayout("Fi8"), DataLayout("Fn8"));
+
+  DataLayout a(""), b("Fi8"), c("Fn8");
+  EXPECT_NE(a, b);
+  EXPECT_NE(a, c);
+  EXPECT_NE(b, c);
+
+  a = b;
+  EXPECT_EQ(a, b);
+  a = c;
+  EXPECT_EQ(a, c);
+}
+
+}  // anonymous namespace

llvm/trunk/unittests/IR/FunctionTest.cpp

TEST(FunctionTest, setSection) {
   F->setSection("");
   EXPECT_FALSE(F->hasSection());
   F->setSection(".text.test");
   F->setSection(".text.test2");
   EXPECT_TRUE(F->getSection() == ".text.test2");
   EXPECT_TRUE(F->hasSection());
 }
 
+TEST(FunctionTest, GetPointerAlignment) {
+  LLVMContext Context;
+  Type *VoidType(Type::getVoidTy(Context));
+  FunctionType *FuncType(FunctionType::get(VoidType, false));
+  Function *Func = Function::Create(
+      FuncType, GlobalValue::ExternalLinkage);
+  EXPECT_EQ(0U, Func->getPointerAlignment(DataLayout("")));
+  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fi8")));
+  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fn8")));
+  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fi16")));
+  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fn16")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fi32")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn32")));
+
+  Func->setAlignment(4U);
+
+  EXPECT_EQ(0U, Func->getPointerAlignment(DataLayout("")));
+  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fi8")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn8")));
+  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fi16")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn16")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fi32")));
+  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn32")));
+}
+
 } // end namespace