wmi updated this revision to Diff 117947.
wmi added a comment.
Address Hal's comment.
Repository:
rL LLVM
https://reviews.llvm.org/D36562
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGRecordLayoutBuilder.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenCXX/finegrain-bitfield-access.cpp
Index: test/CodeGenCXX/finegrain-bitfield-access.cpp
===================================================================
--- test/CodeGenCXX/finegrain-bitfield-access.cpp
+++ test/CodeGenCXX/finegrain-bitfield-access.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \
+// RUN: -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \
+// RUN: -emit-llvm -fsanitize=address -o - %s | FileCheck %s --check-prefix=SANITIZE
+// Check -fsplit-bitfields will be ignored since sanitizer is enabled.
+
+struct S1 {
+ unsigned f1:2;
+ unsigned f2:6;
+ unsigned f3:8;
+ unsigned f4:4;
+ unsigned f5:8;
+};
+
+S1 a1;
+unsigned read8_1() {
+ // CHECK-LABEL: @_Z7read8_1v
+ // CHECK: %bf.load = load i8, i8* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 1), align 1
+ // CHECK-NEXT: %bf.cast = zext i8 %bf.load to i32
+ // CHECK-NEXT: ret i32 %bf.cast
+ // SANITIZE-LABEL: @_Z7read8_1v
+ // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE: %bf.lshr = lshr i32 %bf.load, 8
+ // SANITIZE: %bf.clear = and i32 %bf.lshr, 255
+ // SANITIZE: ret i32 %bf.clear
+ return a1.f3;
+}
+void write8_1() {
+ // CHECK-LABEL: @_Z8write8_1v
+ // CHECK: store i8 3, i8* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 1), align 1
+ // CHECK-NEXT: ret void
+ // SANITIZE-LABEL: @_Z8write8_1v
+ // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE-NEXT: %bf.clear = and i32 %bf.load, -65281
+ // SANITIZE-NEXT: %bf.set = or i32 %bf.clear, 768
+ // SANITIZE-NEXT: store i32 %bf.set, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE-NEXT: ret void
+ a1.f3 = 3;
+}
+
+unsigned read8_2() {
+ // CHECK-LABEL: @_Z7read8_2v
+ // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+ // CHECK-NEXT: %bf.lshr = lshr i16 %bf.load, 4
+ // CHECK-NEXT: %bf.clear = and i16 %bf.lshr, 255
+ // CHECK-NEXT: %bf.cast = zext i16 %bf.clear to i32
+ // CHECK-NEXT: ret i32 %bf.cast
+ // SANITIZE-LABEL: @_Z7read8_2v
+ // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE-NEXT: %bf.lshr = lshr i32 %bf.load, 20
+ // SANITIZE-NEXT: %bf.clear = and i32 %bf.lshr, 255
+ // SANITIZE-NEXT: ret i32 %bf.clear
+ return a1.f5;
+}
+void write8_2() {
+ // CHECK-LABEL: @_Z8write8_2v
+ // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+ // CHECK-NEXT: %bf.clear = and i16 %bf.load, -4081
+ // CHECK-NEXT: %bf.set = or i16 %bf.clear, 48
+ // CHECK-NEXT: store i16 %bf.set, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+ // CHECK-NEXT: ret void
+ // SANITIZE-LABEL: @_Z8write8_2v
+ // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE-NEXT: %bf.clear = and i32 %bf.load, -267386881
+ // SANITIZE-NEXT: %bf.set = or i32 %bf.clear, 3145728
+ // SANITIZE-NEXT: store i32 %bf.set, i32* getelementptr inbounds {{.*}}, align 4
+ // SANITIZE-NEXT: ret void
+ a1.f5 = 3;
+}
+
+struct S2 {
+ unsigned long f1:16;
+ unsigned long f2:16;
+ unsigned long f3:6;
+};
+
+S2 a2;
+unsigned read16_1() {
+ // CHECK-LABEL: @_Z8read16_1v
+ // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 0), align 8
+ // CHECK-NEXT: %bf.cast = zext i16 %bf.load to i64
+ // CHECK-NEXT: %conv = trunc i64 %bf.cast to i32
+ // CHECK-NEXT: ret i32 %conv
+ // SANITIZE-LABEL: @_Z8read16_1v
+ // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.clear = and i64 %bf.load, 65535
+ // SANITIZE-NEXT: %conv = trunc i64 %bf.clear to i32
+ // SANITIZE-NEXT: ret i32 %conv
+ return a2.f1;
+}
+unsigned read16_2() {
+ // CHECK-LABEL: @_Z8read16_2v
+ // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 1), align 2
+ // CHECK-NEXT: %bf.cast = zext i16 %bf.load to i64
+ // CHECK-NEXT: %conv = trunc i64 %bf.cast to i32
+ // CHECK-NEXT: ret i32 %conv
+ // SANITIZE-LABEL: @_Z8read16_2v
+ // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.lshr = lshr i64 %bf.load, 16
+ // SANITIZE-NEXT: %bf.clear = and i64 %bf.lshr, 65535
+ // SANITIZE-NEXT: %conv = trunc i64 %bf.clear to i32
+ // SANITIZE-NEXT: ret i32 %conv
+ return a2.f2;
+}
+
+void write16_1() {
+ // CHECK-LABEL: @_Z9write16_1v
+ // CHECK: store i16 5, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 0), align 8
+ // CHECK-NEXT: ret void
+ // SANITIZE-LABEL: @_Z9write16_1v
+ // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.clear = and i64 %bf.load, -65536
+ // SANITIZE-NEXT: %bf.set = or i64 %bf.clear, 5
+ // SANITIZE-NEXT: store i64 %bf.set, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: ret void
+ a2.f1 = 5;
+}
+void write16_2() {
+ // CHECK-LABEL: @_Z9write16_2v
+ // CHECK: store i16 5, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 1), align 2
+ // CHECK-NEXT: ret void
+ // SANITIZE-LABEL: @_Z9write16_2v
+ // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.clear = and i64 %bf.load, -4294901761
+ // SANITIZE-NEXT: %bf.set = or i64 %bf.clear, 327680
+ // SANITIZE-NEXT: store i64 %bf.set, i64* bitcast {{.*}}, align 8
+ // SANITIZE-NEXT: ret void
+ a2.f2 = 5;
+}
+
+struct S3 {
+ unsigned long f1:14;
+ unsigned long f2:18;
+ unsigned long f3:32;
+};
+
+S3 a3;
+unsigned read32_1() {
+ // CHECK-LABEL: @_Z8read32_1v
+ // CHECK: %bf.load = load i32, i32* getelementptr inbounds (%struct.S3, %struct.S3* @a3, i32 0, i32 1), align 4
+ // CHECK-NEXT: %bf.cast = zext i32 %bf.load to i64
+ // CHECK-NEXT: %conv = trunc i64 %bf.cast to i32
+ // CHECK-NEXT: ret i32 %conv
+ // SANITIZE-LABEL: @_Z8read32_1v
+ // SANITIZE: %bf.load = load i64, i64* getelementptr inbounds {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.lshr = lshr i64 %bf.load, 32
+ // SANITIZE-NEXT: %conv = trunc i64 %bf.lshr to i32
+ // SANITIZE-NEXT: ret i32 %conv
+ return a3.f3;
+}
+void write32_1() {
+ // CHECK-LABEL: @_Z9write32_1v
+ // CHECK: store i32 5, i32* getelementptr inbounds (%struct.S3, %struct.S3* @a3, i32 0, i32 1), align 4
+ // CHECK-NEXT: ret void
+ // SANITIZE-LABEL: @_Z9write32_1v
+ // SANITIZE: %bf.load = load i64, i64* getelementptr inbounds {{.*}}, align 8
+ // SANITIZE-NEXT: %bf.clear = and i64 %bf.load, 4294967295
+ // SANITIZE-NEXT: %bf.set = or i64 %bf.clear, 21474836480
+ // SANITIZE-NEXT: store i64 %bf.set, i64* getelementptr inbounds {{.*}}, align 8
+ // SANITIZE-NEXT: ret void
+ a3.f3 = 5;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -545,6 +545,9 @@
OPT_fuse_register_sized_bitfield_access);
Opts.RelaxedAliasing = Args.hasArg(OPT_relaxed_aliasing);
Opts.StructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa);
+ Opts.FineGrainedBitfieldAccesses =
+ Args.hasFlag(OPT_ffine_grained_bitfield_accesses,
+ OPT_fno_fine_grained_bitfield_accesses, false);
Opts.DwarfDebugFlags = Args.getLastArgValue(OPT_dwarf_debug_flags);
Opts.MergeAllConstants = !Args.hasArg(OPT_fno_merge_all_constants);
Opts.NoCommon = Args.hasArg(OPT_fno_common);
@@ -2747,6 +2750,13 @@
if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64) {
Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
}
+
+ // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.
+ if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
+ !Res.getLangOpts()->Sanitize.empty()) {
+ Res.getCodeGenOpts().FineGrainedBitfieldAccesses = false;
+ Diags.Report(diag::warn_drv_fine_grained_bitfield_accesses_ignored);
+ }
return Success;
}
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3342,6 +3342,9 @@
options::OPT_fno_optimize_sibling_calls))
CmdArgs.push_back("-mdisable-tail-calls");
+ Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
+ options::OPT_fno_fine_grained_bitfield_accesses);
+
// Handle segmented stacks.
if (Args.hasArg(options::OPT_fsplit_stack))
CmdArgs.push_back("-split-stacks");
Index: lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -403,6 +403,26 @@
}
return;
}
+
+ // Check if current Field is better as a single field run. When current field
+ // has legal integer width, and its bitfield offset is naturally aligned, it
+ // is better to make the bitfield a separate storage component so as it can be
+ // accessed directly with lower cost.
+ auto IsBetterAsSingleFieldRun = [&](RecordDecl::field_iterator Field) {
+ if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
+ return false;
+ unsigned Width = Field->getBitWidthValue(Context);
+ if (!DataLayout.isLegalInteger(Width))
+ return false;
+ // Make sure Field is natually aligned if it is treated as an IType integer.
+ if (getFieldBitOffset(*Field) %
+ Context.toBits(getAlignment(getIntNType(Width))) !=
+ 0)
+ return false;
+ return true;
+ };
+
+ // The start of a new run is better as a single field.
for (;;) {
// Check to see if we need to start a new run.
if (Run == FieldEnd) {
@@ -418,13 +438,23 @@
++Field;
continue;
}
- // Add bitfields to the run as long as they qualify.
- if (Field != FieldEnd && Field->getBitWidthValue(Context) != 0 &&
+
+ // If the start field of a new run is better as a single run, or
+ // if current field is better as a single run, or
+ // if current field has zero width bitfield, or
+ // if the offset of current field is inconsistent with the offset of
+ // previous field plus its offset,
+ // skip the block below and go ahead to emit the storage.
+ // Otherwise, try to add bitfields to the run.
+ if (Run != FieldEnd && !IsBetterAsSingleFieldRun(Run) &&
+ Field != FieldEnd && !IsBetterAsSingleFieldRun(Field) &&
+ Field->getBitWidthValue(Context) != 0 &&
Tail == getFieldBitOffset(*Field)) {
Tail += Field->getBitWidthValue(Context);
++Field;
continue;
}
+
// We've hit a break-point in the run and need to emit a storage field.
llvm::Type *Type = getIntNType(Tail - StartBitOffset);
// Add the storage member to the record and set the bitfield info for all of
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -179,6 +179,8 @@
CODEGENOPT(SanitizeStats , 1, 0) ///< Collect statistics for sanitizers.
CODEGENOPT(SimplifyLibCalls , 1, 1) ///< Set when -fbuiltin is enabled.
CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.
+CODEGENOPT(FineGrainedBitfieldAccesses, 1, 0) ///< Use separate access for bitfields
+ ///< with legal widths and alignments.
CODEGENOPT(StrictEnums , 1, 0) ///< Optimize based on strict enum definition.
CODEGENOPT(StrictVTablePointers, 1, 0) ///< Optimize based on the strict vtable pointers
CODEGENOPT(TimePasses , 1, 0) ///< Set when -ftime-report is enabled.
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1036,6 +1036,13 @@
Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Filename defining the whitelist for imbuing the 'never instrument' XRay attribute.">;
+def ffine_grained_bitfield_accesses : Flag<["-"],
+ "ffine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
+ HelpText<"Use separate access for bitfields with legal widths and alignments.">;
+def fno_fine_grained_bitfield_accesses : Flag<["-"],
+ "fno-fine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
+ HelpText<"Use a big integer wrap for a consecutive run of bitfields.">;
+
def flat__namespace : Flag<["-"], "flat_namespace">;
def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group<f_Group>;
def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group<f_Group>;
Index: include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -330,4 +330,8 @@
"unable to find a Visual Studio installation; "
"try running Clang from a developer command prompt">,
InGroup<DiagGroup<"msvc-not-found">>;
+
+def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
+ "option '-ffine-grained-bitfield-accesses' cannot be enabled together with sanitizer; flag ignored">,
+ InGroup<OptionIgnored>;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
