Fixes and simplifications to CPU-data handling (#13881)
In DispatchABI.cpp:
* Was not checking if CPU feature bits in the schema applied to the
target architecture.
* Was not checking if CPU feature bits belonged to field 0 even though
the current code only handles that. Other fields are currently unused
but at least now we will get an assert failure if that changes.
* Was building a vector of bits, then creating one `LLVM::OrOp`for each
bit. Instead, why not OR those bits in C++ and create a single
`LLVM::OrOp` with the resulting OR-ed value?
* Was not checking that the first character of each feature string was a
`'+'` before dropping it.
* Was room for simplification in how the schema was used.
Additionally, folding into this PR a simplification to `iree-cpuinfo` (a
standalone tool to report the host CPU's features exactly as detected by
IREE). It was quadratic-time, as it unnecessarily called
`iree_cpu_lookup_data_by_key` for each feature.
diff --git a/compiler/src/iree/compiler/Codegen/LLVMCPU/DispatchABI.cpp b/compiler/src/iree/compiler/Codegen/LLVMCPU/DispatchABI.cpp
index 25c2ffb..39709b2 100644
--- a/compiler/src/iree/compiler/Codegen/LLVMCPU/DispatchABI.cpp
+++ b/compiler/src/iree/compiler/Codegen/LLVMCPU/DispatchABI.cpp
@@ -6,6 +6,7 @@
#include "iree/compiler/Codegen/LLVMCPU/DispatchABI.h"
+#include "iree/compiler/Codegen/Utils/Utils.h"
#include "iree/schemas/cpu_data.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/Support/CommandLine.h"
@@ -24,39 +25,6 @@
namespace mlir {
namespace iree_compiler {
-// List of all defined llvm feature-name to bit pattern used to represent it.
-// This is derived based on the schema in `runtime/src/iree/schemas/`.
-// TODO(ravishankarm): This link to the runtime schemas needs to be broken.
-// Instead we should use a reflection callback to resolve arch guarded features
-// directly in the compiler.
-
-// Struct to capture tuple of llvm feature-name to bit pattern used to represent
-// it.
-struct iree_llvm_name_and_bit_pattern_t {
- const char *llvm_name;
- unsigned long long bit_pattern;
-};
-
-#define IREE_CPU_FEATURE_BIT_NAME(arch, field_index, bit_name) \
- IREE_CPU_DATA##field_index##_##arch##_##bit_name
-
-#define IREE_CPU_FEATURE_NAME_AND_BIT_PATTERN(arch, field_index, bit_name, \
- llvm_name) \
- {llvm_name, IREE_CPU_FEATURE_BIT_NAME(arch, field_index, bit_name)},
-
-static const struct iree_llvm_name_and_bit_pattern_t
- iree_llvm_name_and_bit_pattern_list[] = {
-
-#define IREE_CPU_FEATURE_BIT(arch, field_index, bit_pos, bit_name, llvm_name) \
- IREE_CPU_FEATURE_NAME_AND_BIT_PATTERN(arch, field_index, bit_name, llvm_name)
-#include "iree/schemas/cpu_feature_bits.inl"
-#undef IREE_CPU_FEATURE_BIT
-
-};
-
-#undef IREE_CPU_FEATURE_NAME_AND_BIT_PATTERN
-#undef IREE_CPU_FEATURE_BIT_NAME
-
//------------------------------------------------------------------------------
// ExecutableLibraryDI
//------------------------------------------------------------------------------
@@ -885,24 +853,47 @@
return processorDataPtrValue;
}
- SmallVector<uint64_t> specifiedFeatureBitPatterns;
+ // Currently requiring all CPU feature bits to be in field 0. Generalize as
+ // needed when other CPU feature fields start to be used.
+ uint64_t specifiedCpuDataField0 = 0;
{
+ // Map llvm feature-name to bit used to represent it in IREE_CPUDATA_FIELD0.
+ //
+ // TODO(ravishankarm): This link to the runtime schemas needs to be broken.
+ // Instead we should use a reflection callback to resolve arch guarded
+ // features directly in the compiler.
llvm::StringMap<uint64_t> featureToBitPattern;
- for (auto [llvmName, bitPattern] : iree_llvm_name_and_bit_pattern_list) {
- featureToBitPattern[llvmName] = bitPattern;
+ auto targetTriple = getTargetTriple(targetAttr);
+ if (!targetTriple) {
+ return processorDataPtrValue;
}
+ std::string targetArchUppercase =
+ StringRef(getIreeArchNameForTargetTriple(targetTriple.value())).upper();
+#define IREE_CPU_FEATURE_BIT(arch, field_index, bit_pos, bit_name, llvm_name) \
+ if (targetArchUppercase == #arch) { \
+ assert(field_index == 0); \
+ featureToBitPattern[llvm_name] = 1ull << bit_pos; \
+ }
+#include "iree/schemas/cpu_feature_bits.inl"
+#undef IREE_CPU_FEATURE_BIT
+
+ // Find CPU features in featureToBitPattern
SmallVector<StringRef> cpuFeatureStrings;
llvm::cast<StringAttr>(cpuFeatures->getValue())
.getValue()
.split(cpuFeatureStrings, ',', /*MakeSplit=*/-1, /*KeepEmpty=*/false);
for (auto featureString : cpuFeatureStrings) {
- if (featureToBitPattern.count(featureString.drop_front())) {
- specifiedFeatureBitPatterns.push_back(
- featureToBitPattern.lookup(featureString.drop_front()));
+ // CPU features are typically prefixed with a +, e.g. +avx,+avx2,+fma.
+ featureString.consume_front("+");
+ // Silently skip unknown CPU features, more flexible for now. Note that
+ // some featurs occurring here are not standard CPU features but internal
+ // things such as the "+reserve-x18" that we add on arm64.
+ if (featureToBitPattern.count(featureString)) {
+ specifiedCpuDataField0 |= featureToBitPattern.lookup(featureString);
}
}
}
- if (specifiedFeatureBitPatterns.empty()) {
+ if (specifiedCpuDataField0 == 0) {
return processorDataPtrValue;
}
@@ -919,11 +910,9 @@
Value srcData0 =
builder.create<LLVM::LoadOp>(loc, i64Ty, processorDataPtrValue);
// Set the specified CPU arch data.
- for (auto bitPattern : specifiedFeatureBitPatterns) {
- Value bitPatternVal = builder.create<LLVM::ConstantOp>(
- loc, i64Ty, builder.getI64IntegerAttr(bitPattern));
- srcData0 = builder.create<LLVM::OrOp>(loc, srcData0, bitPatternVal);
- }
+ Value bitPatternVal = builder.create<LLVM::ConstantOp>(
+ loc, i64Ty, builder.getI64IntegerAttr(specifiedCpuDataField0));
+ srcData0 = builder.create<LLVM::OrOp>(loc, srcData0, bitPatternVal);
builder.create<LLVM::StoreOp>(loc, srcData0, alloca);
// Copy over the rest.
for (int64_t i = 1, e = ProcessorDataCapacity; i < e; ++i) {
diff --git a/compiler/src/iree/compiler/Codegen/LLVMCPU/test/convert_to_llvm.mlir b/compiler/src/iree/compiler/Codegen/LLVMCPU/test/convert_to_llvm.mlir
index 7acae65..3a3cb25 100644
--- a/compiler/src/iree/compiler/Codegen/LLVMCPU/test/convert_to_llvm.mlir
+++ b/compiler/src/iree/compiler/Codegen/LLVMCPU/test/convert_to_llvm.mlir
@@ -102,13 +102,7 @@
}
// CHECK: llvm.func @paramstruct_cconv_with_extra_fields_and_executable_target(!llvm.ptr)
// CHECK: llvm.func @bar
-// CHECK-DAG: %[[F16C:.+]] = llvm.mlir.constant(16384 : i64) : i64
-// CHECK-DAG: %[[FMA:.+]] = llvm.mlir.constant(2048 : i64) : i64
-// CHECK-DAG: %[[AVX2:.+]] = llvm.mlir.constant(32768 : i64) : i64
-// CHECK-DAG: %[[AVX:.+]] = llvm.mlir.constant(1024 : i64) : i64
-// CHECK-DAG: %[[SSE41:.+]] = llvm.mlir.constant(4 : i64) : i64
-// CHECK-DAG: %[[SSSE3:.+]] = llvm.mlir.constant(2 : i64) : i64
-// CHECK-DAG: %[[SSE3:.+]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK-DAG: %[[CPUDATA_FIELD0:.+]] = llvm.mlir.constant(52239 : i64) : i64
// CHECK-DAG: %[[C8:.+]] = llvm.mlir.constant(8 : i64) : i64
// CHECK-DAG: %[[C1:.+]] = llvm.mlir.constant(1 : index) : i64
// CHECK-DAG: %[[Ci32:.+]] = llvm.mlir.constant(42 : i32) : i32
@@ -117,15 +111,8 @@
// CHECK-DAG: %[[DATA_PTR:.+]] = llvm.getelementptr inbounds %arg0[4]
// CHECK: %[[PROCESSOR_DATA_ALLOCA:.+]] = llvm.alloca %[[C8]] x i64 {alignment = 8 : i64}
// CHECK-DAG: %[[DATA:.+]] = llvm.load %[[DATA_PTR]]
-// CHECK-DAG: %[[OR0:.+]] = llvm.or %[[DATA]], %[[SSE3]]
-// CHECK-DAG: %[[OR1:.+]] = llvm.or %[[OR0]], %[[SSSE3]]
-// CHECK-DAG: %[[OR2:.+]] = llvm.or %[[OR1]], %[[SSE41]]
-// CHECK-DAG: %[[OR3:.+]] = llvm.or %[[OR2]], %[[C8]]
-// CHECK-DAG: %[[OR4:.+]] = llvm.or %[[OR3]], %[[AVX]]
-// CHECK-DAG: %[[OR5:.+]] = llvm.or %[[OR4]], %[[AVX2]]
-// CHECK-DAG: %[[OR6:.+]] = llvm.or %[[OR5]], %[[FMA]]
-// CHECK-DAG: %[[OR7:.+]] = llvm.or %[[OR6]], %[[F16C]]
-// CHECK: llvm.store %[[OR7]], %[[PROCESSOR_DATA_ALLOCA]]
+// CHECK-DAG: %[[OR0:.+]] = llvm.or %[[DATA]], %[[CPUDATA_FIELD0]]
+// CHECK: llvm.store %[[OR0]], %[[PROCESSOR_DATA_ALLOCA]]
// CHECK: %[[PROCESSOR_DATA_PTR_1:.+]] = llvm.getelementptr inbounds %[[DATA_PTR]][1]
// CHECK: %[[PROCESSOR_DATA_1:.+]] = llvm.load %[[PROCESSOR_DATA_PTR_1]]
diff --git a/tools/iree-cpuinfo.c b/tools/iree-cpuinfo.c
index d740465..f5324d0 100644
--- a/tools/iree-cpuinfo.c
+++ b/tools/iree-cpuinfo.c
@@ -11,11 +11,11 @@
int main(int argc, char *argv[]) {
iree_cpu_initialize(iree_allocator_system());
+ const uint64_t* cpu_data = iree_cpu_data_fields();
#define IREE_CPU_FEATURE_BIT(arch, field_index, bit_pos, bit_name, llvm_name) \
if (IREE_ARCH_ENUM == IREE_ARCH_ENUM_##arch) { \
- int64_t result = 0; \
- IREE_CHECK_OK(iree_cpu_lookup_data_by_key(IREE_SV(llvm_name), &result)); \
+ bool result = (cpu_data[field_index] & (1ull << bit_pos)) != 0; \
printf("%-20s %ld\n", llvm_name, result); \
}
#include "iree/schemas/cpu_feature_bits.inl"
