From 92dd321a1479431c894aeb7fac7757f5bf035c52 Mon Sep 17 00:00:00 2001 From: Mitch Phillips Date: Thu, 7 Mar 2019 18:13:39 +0000 Subject: [PATCH] Rollback of rL355585. Introduces memory leak in FunctionTest.GetPointerAlignment that breaks sanitizer buildbots: ``` ================================================================= ==2453==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 1 object(s) allocated from: #0 0x610428 in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:105 #1 0x16936bc in llvm::User::operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/lib/IR/User.cpp:151:19 #2 0x7c3fe9 in Create /b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/IR/Function.h:144:12 #3 0x7c3fe9 in (anonymous namespace)::FunctionTest_GetPointerAlignment_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/IR/FunctionTest.cpp:136 #4 0x1a836a0 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc #5 0x1a836a0 in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2474 #6 0x1a85c55 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11 #7 0x1a870d0 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28 #8 0x1aa5b84 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43 #9 0x1aa4d30 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc #10 0x1aa4d30 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4257 #11 0x1a6b656 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46 #12 0x1a6b656 in main /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50 #13 0x7f5af37a22e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0) Indirect leak of 40 byte(s) in 1 object(s) allocated from: #0 0x610428 in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:105 #1 0x151be6b in make_unique /b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/ADT/STLExtras.h:1349:29 #2 0x151be6b in llvm::Function::Function(llvm::FunctionType*, llvm::GlobalValue::LinkageTypes, unsigned int, llvm::Twine const&, llvm::Module*) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/lib/IR/Function.cpp:241 #3 0x7c4006 in Create /b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/IR/Function.h:144:16 #4 0x7c4006 in (anonymous namespace)::FunctionTest_GetPointerAlignment_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/IR/FunctionTest.cpp:136 #5 0x1a836a0 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc #6 0x1a836a0 in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2474 #7 0x1a85c55 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11 #8 0x1a870d0 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28 #9 0x1aa5b84 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43 #10 0x1aa4d30 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc #11 0x1aa4d30 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4257 #12 0x1a6b656 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46 #13 0x1a6b656 in main /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50 #14 0x7f5af37a22e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0) SUMMARY: AddressSanitizer: 168 byte(s) leaked in 2 allocation(s). ``` See http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/11358/steps/check-llvm%20asan/logs/stdio for more information. Also introduces use-of-uninitialized-value in ConstantsTest.FoldGlobalVariablePtr: ``` ==7070==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x14e703c in User /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/User.h:79:5 #1 0x14e703c in Constant /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/Constant.h:44 #2 0x14e703c in llvm::GlobalValue::GlobalValue(llvm::Type*, llvm::Value::ValueTy, llvm::Use*, unsigned int, llvm::GlobalValue::LinkageTypes, llvm::Twine const&, unsigned int) /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/GlobalValue.h:78 #3 0x14e5467 in GlobalObject /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/GlobalObject.h:34:9 #4 0x14e5467 in llvm::GlobalVariable::GlobalVariable(llvm::Type*, bool, llvm::GlobalValue::LinkageTypes, llvm::Constant*, llvm::Twine const&, llvm::GlobalValue::ThreadLocalMode, unsigned int, bool) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/Globals.cpp:314 #5 0x6938f1 in llvm::(anonymous namespace)::ConstantsTest_FoldGlobalVariablePtr_Test::TestBody() /b/sanitizer-x86_64-linux-fast/build/llvm/unittests/IR/ConstantsTest.cpp:565:18 #6 0x1a240a1 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc #7 0x1a240a1 in testing::Test::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474 #8 0x1a26d26 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11 #9 0x1a2815f in testing::TestCase::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28 #10 0x1a43de8 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43 #11 0x1a42c47 in HandleExceptionsInMethodIfSupported /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc #12 0x1a42c47 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257 #13 0x1a0dfba in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46 #14 0x1a0dfba in main /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50 #15 0x7f2081c412e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0) #16 0x4dff49 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/unittests/IR/IRTests+0x4dff49) SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/User.h:79:5 in User ``` See http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/30222/steps/check-llvm%20msan/logs/stdio for more information. llvm-svn: 355616 --- clang/lib/Basic/Targets/ARM.cpp | 23 +++--- clang/test/CodeGen/armv7k-abi.c | 2 +- clang/test/CodeGen/target-data.c | 10 +-- llvm/docs/LangRef.rst | 8 -- llvm/include/llvm/IR/DataLayout.h | 23 ------ llvm/lib/IR/ConstantFold.cpp | 28 +------ llvm/lib/IR/DataLayout.cpp | 20 ----- llvm/lib/IR/Value.cpp | 12 +-- llvm/lib/Target/ARM/ARMTargetMachine.cpp | 4 - llvm/unittests/IR/CMakeLists.txt | 1 - llvm/unittests/IR/ConstantsTest.cpp | 101 ----------------------- llvm/unittests/IR/DataLayoutTest.cpp | 47 ----------- llvm/unittests/IR/FunctionTest.cpp | 25 ------ 13 files changed, 25 insertions(+), 279 deletions(-) delete mode 100644 llvm/unittests/IR/DataLayoutTest.cpp diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp index 2273c1cdf4b480..61a520bd5e0b0f 100644 --- a/clang/lib/Basic/Targets/ARM.cpp +++ b/clang/lib/Basic/Targets/ARM.cpp @@ -40,14 +40,13 @@ void ARMTargetInfo::setABIAAPCS() { // so set preferred for small types to 32. if (T.isOSBinFormatMachO()) { resetDataLayout(BigEndian - ? "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"); + ? "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"); } 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" @@ -55,11 +54,11 @@ void ARMTargetInfo::setABIAAPCS() { "-S64"); } else if (T.isOSNaCl()) { assert(!BigEndian && "NaCl on ARM does not support big endian"); - resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S128"); + resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128"); } else { resetDataLayout(BigEndian - ? "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"); + ? "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"); } // FIXME: Enumerated types are variable width in straight AAPCS. @@ -88,17 +87,17 @@ void ARMTargetInfo::setABIAPCS(bool IsAAPCS16) { if (T.isOSBinFormatMachO() && IsAAPCS16) { assert(!BigEndian && "AAPCS16 does not support big-endian"); - resetDataLayout("e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128"); + resetDataLayout("e-m:o-p:32:32-i64:64-a:0:32-n32-S128"); } else if (T.isOSBinFormatMachO()) resetDataLayout( BigEndian - ? "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"); + ? "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"); else resetDataLayout( BigEndian - ? "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"); + ? "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"); // FIXME: Override "preferred align" for double and long long. } @@ -1056,7 +1055,7 @@ CygwinARMTargetInfo::CygwinARMTargetInfo(const llvm::Triple &Triple, this->WCharType = TargetInfo::UnsignedShort; TLSSupported = false; DoubleAlign = LongLongAlign = 64; - resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"); + resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"); } void CygwinARMTargetInfo::getTargetDefines(const LangOptions &Opts, diff --git a/clang/test/CodeGen/armv7k-abi.c b/clang/test/CodeGen/armv7k-abi.c index c17bc9d2f29305..9b57de8727bd73 100644 --- a/clang/test/CodeGen/armv7k-abi.c +++ b/clang/test/CodeGen/armv7k-abi.c @@ -4,7 +4,7 @@ // Make sure 64 and 128 bit types are naturally aligned by the v7k ABI: -// CHECK: target datalayout = "e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128" +// CHECK: target datalayout = "e-m:o-p:32:32-i64:64-a:0:32-n32-S128" typedef struct { float arr[4]; diff --git a/clang/test/CodeGen/target-data.c b/clang/test/CodeGen/target-data.c index 11f3767a04b656..0c2b1e4cfff361 100644 --- a/clang/test/CodeGen/target-data.c +++ b/clang/test/CodeGen/target-data.c @@ -96,7 +96,7 @@ // 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-Fi8-i64:64-v128:64:128-a:0:32-n32-S128" +// ARM-NACL: target datalayout = "e-m:e-p:32:32-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 @@ -165,19 +165,19 @@ // 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-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" +// THUMB: target datalayout = "e-m:e-p:32:32-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-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" +// ARM: target datalayout = "e-m:e-p:32:32-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-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32" +// THUMB-GNU: target datalayout = "e-m:e-p:32:32-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-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32" +// ARM-GNU: target datalayout = "e-m:e-p:32:32-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 diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 261d1d47d20ea8..86972acf6ad379 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2004,14 +2004,6 @@ as follows: targets. ``a::`` This specifies the alignment for an object of aggregate type. -``F`` - This specifies the alignment for function pointers. - The options for ```` are: - - * ``i``: The alignment of function pointers is independent of the alignment - of functions, and is a multiple of ````. - * ``n``: The alignment of function pointers is a multiple of the explicit - alignment specified on the function, and is a multiple of ````. ``m:`` If present, specifies that llvm names are mangled in the output. Symbols prefixed with the mangling escape character ``\01`` are passed through diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h index 05c72f10d30968..869b27b1fdaf42 100644 --- a/llvm/include/llvm/IR/DataLayout.h +++ b/llvm/include/llvm/IR/DataLayout.h @@ -108,13 +108,6 @@ struct PointerAlignElem { /// 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; @@ -123,9 +116,6 @@ class DataLayout { unsigned StackNaturalAlign; unsigned ProgramAddrSpace; - unsigned FunctionPtrAlign; - FunctionPtrAlignType TheFunctionPtrAlignType; - enum ManglingModeT { MM_None, MM_ELF, @@ -209,8 +199,6 @@ class DataLayout { BigEndian = DL.isBigEndian(); AllocaAddrSpace = DL.AllocaAddrSpace; StackNaturalAlign = DL.StackNaturalAlign; - FunctionPtrAlign = DL.FunctionPtrAlign; - TheFunctionPtrAlignType = DL.TheFunctionPtrAlignType; ProgramAddrSpace = DL.ProgramAddrSpace; ManglingMode = DL.ManglingMode; LegalIntWidths = DL.LegalIntWidths; @@ -268,17 +256,6 @@ class DataLayout { 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 { diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp index 3784405ecb0878..037f120f47db99 100644 --- a/llvm/lib/IR/ConstantFold.cpp +++ b/llvm/lib/IR/ConstantFold.cpp @@ -26,7 +26,6 @@ #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" @@ -1077,29 +1076,10 @@ Constant *llvm::ConstantFoldBinaryInstruction(unsigned Opcode, Constant *C1, isa(CE1->getOperand(0))) { GlobalValue *GV = cast(CE1->getOperand(0)); - 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(GV)) - GVAlign = 4U; - } else if (isa(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(); - } + // Functions are at least 4-byte aligned. + unsigned GVAlign = GV->getAlignment(); + if (isa(GV)) + GVAlign = std::max(GVAlign, 4U); if (GVAlign > 1) { unsigned DstWidth = CI2->getType()->getBitWidth(); diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp index 943f5381c64971..2644214a49c0d1 100644 --- a/llvm/lib/IR/DataLayout.cpp +++ b/llvm/lib/IR/DataLayout.cpp @@ -184,8 +184,6 @@ void DataLayout::reset(StringRef Desc) { AllocaAddrSpace = 0; StackNaturalAlign = 0; ProgramAddrSpace = 0; - FunctionPtrAlign = 0; - TheFunctionPtrAlignType = FunctionPtrAlignType::Independent; ManglingMode = MM_None; NonIntegralAddressSpaces.clear(); @@ -381,22 +379,6 @@ void DataLayout::parseSpecifier(StringRef Desc) { 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; @@ -450,8 +432,6 @@ bool DataLayout::operator==(const DataLayout &Other) const { 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; diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp index cf32a66c90143c..38eed76fe452c4 100644 --- a/llvm/lib/IR/Value.cpp +++ b/llvm/lib/IR/Value.cpp @@ -648,14 +648,10 @@ unsigned Value::getPointerAlignment(const DataLayout &DL) const { unsigned Align = 0; if (auto *GO = dyn_cast(this)) { - if (isa(GO)) { - switch (DL.getFunctionPtrAlignType()) { - case DataLayout::FunctionPtrAlignType::Independent: - return DL.getFunctionPtrAlign(); - case DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign: - return std::max(DL.getFunctionPtrAlign(), GO->getAlignment()); - } - } + // Don't make any assumptions about function pointer alignment. Some + // targets use the LSBs to store additional information. + if (isa(GO)) + return 0; Align = GO->getAlignment(); if (Align == 0) { if (auto *GVar = dyn_cast(GO)) { diff --git a/llvm/lib/Target/ARM/ARMTargetMachine.cpp b/llvm/lib/Target/ARM/ARMTargetMachine.cpp index 401843c1e0fb14..9954eee2e5f4dc 100644 --- a/llvm/lib/Target/ARM/ARMTargetMachine.cpp +++ b/llvm/lib/Target/ARM/ARMTargetMachine.cpp @@ -141,10 +141,6 @@ static std::string computeDataLayout(const Triple &TT, StringRef CPU, // 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"; diff --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt index c3cdf90c0ef0dd..a823407169f5ed 100644 --- a/llvm/unittests/IR/CMakeLists.txt +++ b/llvm/unittests/IR/CMakeLists.txt @@ -13,7 +13,6 @@ add_llvm_unittest(IRTests CFGBuilder.cpp ConstantRangeTest.cpp ConstantsTest.cpp - DataLayoutTest.cpp DebugInfoTest.cpp DebugTypeODRUniquingTest.cpp DominatorTreeTest.cpp diff --git a/llvm/unittests/IR/ConstantsTest.cpp b/llvm/unittests/IR/ConstantsTest.cpp index 34ebd59bc3d213..6b26b38e35e9ca 100644 --- a/llvm/unittests/IR/ConstantsTest.cpp +++ b/llvm/unittests/IR/ConstantsTest.cpp @@ -475,106 +475,5 @@ TEST(ConstantsTest, BitcastToGEP) { ASSERT_EQ(cast(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)); - - std::unique_ptr Global( - new GlobalVariable(IntType, true, GlobalValue::ExternalLinkage)); - - Global->setAlignment(4); - - ConstantInt *TheConstant(ConstantInt::get(IntType, 2)); - - Constant *TheConstantExpr( - ConstantExpr::getPtrToInt(Global.get(), IntType)); - - ASSERT_TRUE(ConstantExpr::get( \ - Instruction::And, TheConstantExpr, TheConstant)->isNullValue()); -} - } // end anonymous namespace } // end namespace llvm diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp deleted file mode 100644 index e7ed70b7de5d54..00000000000000 --- a/llvm/unittests/IR/DataLayoutTest.cpp +++ /dev/null @@ -1,47 +0,0 @@ -//===- 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 diff --git a/llvm/unittests/IR/FunctionTest.cpp b/llvm/unittests/IR/FunctionTest.cpp index c68b064afe3063..10a359ff64bf5f 100644 --- a/llvm/unittests/IR/FunctionTest.cpp +++ b/llvm/unittests/IR/FunctionTest.cpp @@ -129,29 +129,4 @@ TEST(FunctionTest, setSection) { 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