diff options
author | David Gross <dgross@google.com> | 2017-03-27 15:20:48 -0700 |
---|---|---|
committer | David Gross <dgross@google.com> | 2017-03-28 19:12:47 +0000 |
commit | 8281b5d72ff27077c461aa22e5c51a3a69e25916 (patch) | |
tree | 0a5abc951e9de946adc22dfb057c6da22aae9def | |
parent | b2c126c9a2560f1ea230044888eba6205dd96ba8 (diff) | |
download | libbcc-8281b5d72ff27077c461aa22e5c51a3a69e25916.tar.gz |
Structure layout logic cleanup.
Do not run special x86-32 transformations when slang has explicitly
padded structs.
Verify that front end (Module) and back end (TargetMachine) agree on
the layout of every exported struct type.
Cannot build without slang change:
https://android-review.googlesource.com/#/c/299366/
Bug: http://b/29154200
Bug: http://b/28070272
Test: (aosp_x86-eng emulator, full_fugu-eng, aosp_angler-eng) x
(RsTest 32-bit, RsTest 64-bit, cts -m RenderscriptTest)
tests/run-lit-tests.sh
Tried (unmodified slang, modified bcc) and
( modified slang, unmodified bcc) and
( modified slang, modified bcc)
By instrumenting modified bcc, confimed that:
- Special x8632 layout transformations only run with unmodified slang,
and only when test is compiled for x8632.
"Modified slang" is a forthcoming slang change to add explicit padding
to struct types:
https://android-review.googlesource.com/#/c/299366/
Change-Id: I536497d1152995bf93a48dc83527d1575d5f947e
-rw-r--r-- | include/bcc/Compiler.h | 4 | ||||
-rw-r--r-- | include/bcc/Script.h | 6 | ||||
-rw-r--r-- | lib/Android.bp | 2 | ||||
-rw-r--r-- | lib/Compiler.cpp | 87 | ||||
-rw-r--r-- | lib/RSCompilerDriver.cpp | 6 | ||||
-rw-r--r-- | lib/RSKernelExpand.cpp | 24 | ||||
-rw-r--r-- | tools/bcc/Android.bp | 2 | ||||
-rw-r--r-- | tools/bcc_compat/Android.bp | 3 |
8 files changed, 116 insertions, 18 deletions
diff --git a/include/bcc/Compiler.h b/include/bcc/Compiler.h index 73f8274..708e1b0 100644 --- a/include/bcc/Compiler.h +++ b/include/bcc/Compiler.h @@ -67,7 +67,9 @@ public: kIllegalGlobalFunction, - kErrInvalidTargetMachine + kErrInvalidTargetMachine, + + kErrInvalidLayout }; static const char *GetErrorString(enum ErrorCode pErrCode); diff --git a/include/bcc/Script.h b/include/bcc/Script.h index b4135c5..3b10d82 100644 --- a/include/bcc/Script.h +++ b/include/bcc/Script.h @@ -17,6 +17,8 @@ #ifndef BCC_SCRIPT_H #define BCC_SCRIPT_H +#include "slang_version.h" + #include <llvm/Support/CodeGen.h> #include "bcc/Source.h" @@ -65,6 +67,10 @@ public: return getSource().getCompilerVersion(); } + bool isStructExplicitlyPaddedBySlang() const { + return getCompilerVersion() >= SlangVersion::N_STRUCT_EXPLICIT_PADDING; + } + void setOptimizationLevel(llvm::CodeGenOpt::Level pOptimizationLevel) { mOptimizationLevel = pOptimizationLevel; } diff --git a/lib/Android.bp b/lib/Android.bp index e16b034..6ac3652 100644 --- a/lib/Android.bp +++ b/lib/Android.bp @@ -43,6 +43,8 @@ cc_library_shared { shared_libs: ["libbcinfo"], + header_libs: ["slang_headers"], + target: { windows: { enabled: true, diff --git a/lib/Compiler.cpp b/lib/Compiler.cpp index ca14d1f..5000fa1 100644 --- a/lib/Compiler.cpp +++ b/lib/Compiler.cpp @@ -45,6 +45,74 @@ #include <string> #include <set> +namespace { + +// Name of metadata node where list of exported types resides +// (should be synced with slang_rs_metadata.h) +static const llvm::StringRef ExportedTypeMetadataName = "#rs_export_type"; + +// Every exported struct type must have the same layout according to +// the Module's DataLayout that it does according to the +// TargetMachine's DataLayout -- that is, the front end (represented +// by Module) and back end (represented by TargetMachine) must agree. +bool validateLayoutOfExportedTypes(const llvm::Module &module, + const llvm::DataLayout &moduleDataLayout, + const llvm::DataLayout &targetDataLayout) { + if (moduleDataLayout == targetDataLayout) + return true; + + const llvm::NamedMDNode *const exportedTypesMD = + module.getNamedMetadata(ExportedTypeMetadataName); + if (!exportedTypesMD) + return true; + + bool allOk = true; + for (const llvm::MDNode *const exportedTypeMD : exportedTypesMD->operands()) { + bccAssert(exportedTypeMD->getNumOperands() == 1); + + // The name of the type in LLVM is the name of the type in the + // metadata with "struct." prepended. + std::string exportedTypeName = + "struct." + + llvm::cast<llvm::MDString>(exportedTypeMD->getOperand(0))->getString().str(); + + llvm::StructType *const exportedType = module.getTypeByName(exportedTypeName); + + if (!exportedType) { + // presumably this means the type got optimized away + continue; + } + + const llvm::StructLayout *const moduleStructLayout = moduleDataLayout.getStructLayout(exportedType); + const llvm::StructLayout *const targetStructLayout = targetDataLayout.getStructLayout(exportedType); + + if (moduleStructLayout->getSizeInBits() != targetStructLayout->getSizeInBits()) { + ALOGE("%s: getSizeInBits() does not match (%u, %u)", exportedTypeName.c_str(), + unsigned(moduleStructLayout->getSizeInBits()), unsigned(targetStructLayout->getSizeInBits())); + allOk = false; + } + + // We deliberately do not check alignment of the struct as a whole -- the explicit padding + // from slang doesn't force the alignment. + + for (unsigned elementCount = exportedType->getNumElements(), elementIdx = 0; + elementIdx < elementCount; ++elementIdx) { + if (moduleStructLayout->getElementOffsetInBits(elementIdx) != + targetStructLayout->getElementOffsetInBits(elementIdx)) { + ALOGE("%s: getElementOffsetInBits(%u) does not match (%u, %u)", + exportedTypeName.c_str(), elementIdx, + unsigned(moduleStructLayout->getElementOffsetInBits(elementIdx)), + unsigned(targetStructLayout->getElementOffsetInBits(elementIdx))); + allOk = false; + } + } + } + + return allOk; +} + +} // end unnamed namespace + using namespace bcc; const char *Compiler::GetErrorString(enum ErrorCode pErrCode) { @@ -77,6 +145,8 @@ const char *Compiler::GetErrorString(enum ErrorCode pErrCode) { return "Use of undefined external function"; case kErrInvalidTargetMachine: return "Invalid/unexpected llvm::TargetMachine."; + case kErrInvalidLayout: + return "Invalid layout (RenderScript ABI and native ABI are incompatible)"; } // This assert should never be reached as the compiler verifies that the @@ -252,12 +322,17 @@ enum Compiler::ErrorCode Compiler::compile(Script &script, return kErrInvalidSource; } - if (getTargetMachine().getTargetTriple().getArch() == llvm::Triple::x86) { - // Detect and fail if TargetMachine datalayout is different than what we - // expect. This is to detect changes in default target layout for x86 and - // update X86_CUSTOM_DL_STRING in include/bcc/Config/Config.h appropriately. - if (dl.getStringRepresentation().compare(X86_DEFAULT_DL_STRING) != 0) { - return kErrInvalidTargetMachine; + if (script.isStructExplicitlyPaddedBySlang()) { + if (!validateLayoutOfExportedTypes(module, module.getDataLayout(), dl)) + return kErrInvalidLayout; + } else { + if (getTargetMachine().getTargetTriple().getArch() == llvm::Triple::x86) { + // Detect and fail if TargetMachine datalayout is different than what we + // expect. This is to detect changes in default target layout for x86 and + // update X86_CUSTOM_DL_STRING in include/bcc/Config/Config.h appropriately. + if (dl.getStringRepresentation().compare(X86_DEFAULT_DL_STRING) != 0) { + return kErrInvalidTargetMachine; + } } } diff --git a/lib/RSCompilerDriver.cpp b/lib/RSCompilerDriver.cpp index 6ed7400..be91343 100644 --- a/lib/RSCompilerDriver.cpp +++ b/lib/RSCompilerDriver.cpp @@ -20,6 +20,7 @@ #include "FileMutex.h" #include "Log.h" #include "RSScriptGroupFusion.h" +#include "slang_version.h" #include "bcc/BCCContext.h" #include "bcc/Compiler.h" @@ -130,7 +131,8 @@ Compiler::ErrorCode RSCompilerDriver::compileScript(Script& pScript, const char* // (during LinkRuntime below) to ensure that RenderScript-driver-provided // structs (like Allocation_t) don't get forced into using the ARM layout // rules. - if (mCompiler.getTargetMachine().getTargetTriple().getArch() == llvm::Triple::x86) { + if (!pScript.isStructExplicitlyPaddedBySlang() && + (mCompiler.getTargetMachine().getTargetTriple().getArch() == llvm::Triple::x86)) { mCompiler.translateGEPs(pScript); } @@ -275,7 +277,7 @@ bool RSCompilerDriver::build(BCCContext &pContext, // Assertion-enabled builds can't compile legacy bitcode (due to the use of // getName() with anonymous structure definitions). #ifdef _DEBUG - static const uint32_t kSlangMinimumFixedStructureNames = 2310; + static const uint32_t kSlangMinimumFixedStructureNames = SlangVersion::M_RS_OBJECT; uint32_t version = wrapper.getCompilerVersion(); if (version < kSlangMinimumFixedStructureNames) { ALOGE("Found invalid legacy bitcode compiled with a version %u llvm-rs-cc " diff --git a/lib/RSKernelExpand.cpp b/lib/RSKernelExpand.cpp index e2fb6f4..2f451d7 100644 --- a/lib/RSKernelExpand.cpp +++ b/lib/RSKernelExpand.cpp @@ -22,6 +22,8 @@ #include "bcc/Config.h" #include "bcinfo/MetadataExtractor.h" +#include "slang_version.h" + #include <cstdlib> #include <functional> #include <unordered_set> @@ -89,7 +91,7 @@ public: static char ID; private: - static const size_t RS_KERNEL_INPUT_LIMIT = 8; // see frameworks/base/libs/rs/cpu_ref/rsCpuCoreRuntime.h + static const size_t RS_KERNEL_INPUT_LIMIT = 8; // see frameworks/base/libs/rs/cpu_ref/rsCpuCoreRuntime.h typedef std::unordered_set<llvm::Function *> FunctionSet; @@ -130,6 +132,8 @@ private: llvm::FunctionType *ExpandedForEachType; llvm::Type *RsExpandKernelDriverInfoPfxTy; + // Initialized when we begin to process each Module + bool mStructExplicitlyPaddedBySlang; uint32_t mExportForEachCount; const char **mExportForEachNameList; const uint32_t *mExportForEachSignatureList; @@ -674,7 +678,7 @@ public: llvm::LoadInst *InBufPtr = Builder.CreateLoad(InBufPtrAddr, "input_buf"); llvm::Value *CastInBufPtr = nullptr; - if (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING) { + if (mStructExplicitlyPaddedBySlang || (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING)) { CastInBufPtr = Builder.CreatePointerCast(InBufPtr, InType, "casted_in"); } else { // The disagreement between module and x86 target machine datalayout @@ -682,7 +686,7 @@ public: // code and bcc codegen for GetElementPtr. To solve this issue, skip the // cast to InType and leave CastInBufPtr as an int8_t*. The buffer is // later indexed with an explicit byte offset computed based on - // X86_CUSTOM_DL_STRING and then bitcast it to actual input type. + // X86_CUSTOM_DL_STRING and then bitcast to actual input type. CastInBufPtr = InBufPtr; } @@ -728,7 +732,7 @@ public: for (size_t Index = 0; Index < NumInputs; ++Index) { llvm::Value *InPtr = nullptr; - if (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING) { + if (mStructExplicitlyPaddedBySlang || (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING)) { InPtr = Builder.CreateInBoundsGEP(InBufPtrs[Index], Offset); } else { // Treat x86 input buffer as byte[], get indexed pointer with explicit @@ -784,7 +788,7 @@ public: } llvm::DataLayout DL(Module); - if (Module->getTargetTriple() == DEFAULT_X86_TRIPLE_STRING) { + if (!mStructExplicitlyPaddedBySlang && (Module->getTargetTriple() == DEFAULT_X86_TRIPLE_STRING)) { DL.reset(X86_CUSTOM_DL_STRING); } @@ -917,7 +921,7 @@ public: // TODO: Refactor this to share functionality with ExpandOldStyleForEach. llvm::DataLayout DL(Module); - if (Module->getTargetTriple() == DEFAULT_X86_TRIPLE_STRING) { + if (!mStructExplicitlyPaddedBySlang && (Module->getTargetTriple() == DEFAULT_X86_TRIPLE_STRING)) { DL.reset(X86_CUSTOM_DL_STRING); } llvm::Type *Int32Ty = llvm::Type::getInt32Ty(*Context); @@ -1001,7 +1005,7 @@ public: OutBasePtr->setMetadata("tbaa", TBAAPointer); } - if (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING) { + if (mStructExplicitlyPaddedBySlang || (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING)) { CastedOutBasePtr = Builder.CreatePointerCast(OutBasePtr, OutTy, "casted_out"); } else { // The disagreement between module and x86 target machine datalayout @@ -1009,7 +1013,7 @@ public: // code and bcc codegen for GetElementPtr. To solve this issue, skip the // cast to OutTy and leave CastedOutBasePtr as an int8_t*. The buffer // is later indexed with an explicit byte offset computed based on - // X86_CUSTOM_DL_STRING and then bitcast it to actual output type. + // X86_CUSTOM_DL_STRING and then bitcast to actual output type. CastedOutBasePtr = OutBasePtr; } } @@ -1053,7 +1057,7 @@ public: if (CastedOutBasePtr) { llvm::Value *OutOffset = Builder.CreateSub(IV, Arg_x1); - if (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING) { + if (mStructExplicitlyPaddedBySlang || (Module->getTargetTriple() != DEFAULT_X86_TRIPLE_STRING)) { OutPtr = Builder.CreateInBoundsGEP(CastedOutBasePtr, OutOffset); } else { // Treat x86 output buffer as byte[], get indexed pointer with explicit @@ -1374,6 +1378,8 @@ public: return false; } + mStructExplicitlyPaddedBySlang = (me.getCompilerVersion() >= SlangVersion::N_STRUCT_EXPLICIT_PADDING); + // Expand forEach_* style kernels. mExportForEachCount = me.getExportForEachSignatureCount(); mExportForEachNameList = me.getExportForEachNameList(); diff --git a/tools/bcc/Android.bp b/tools/bcc/Android.bp index 791d498..507fdd9 100644 --- a/tools/bcc/Android.bp +++ b/tools/bcc/Android.bp @@ -27,6 +27,8 @@ cc_binary { "libLLVM", ], + header_libs: ["slang_headers"], + target: { host: { host_ldlibs: ["-ldl"], diff --git a/tools/bcc_compat/Android.bp b/tools/bcc_compat/Android.bp index 51f5a81..12e703e 100644 --- a/tools/bcc_compat/Android.bp +++ b/tools/bcc_compat/Android.bp @@ -33,12 +33,15 @@ cc_binary_host { host_ldlibs: ["-ldl"], }, }, + shared_libs: [ "libbcc", "libbcinfo", "libLLVM", ], + header_libs: ["slang_headers"], + product_variables: { unbundled_build: { // Don't build for unbundled branches |