llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-hexagon Author: Pengcheng Wang (wangpc-pp) <details> <summary>Changes</summary> Introduce a `TargetOptions::EnableStoreMerging` flag (default true) that both `DAGCombiner::mergeConsecutiveStores` (SelectionDAG) and the `GlobalISel`'s `LoadStoreOpt` pass honor to control store merging. Wire it up in Clang via a new `CodeGenOpts::StoreMerging` option and a GCC-compatible `-fstore-merging/-fno-store-merging` driver flag, so that `-fno-store-merging` disables merging of adjacent stores into a wider store across both instruction selectors. Add a `-store-merging` command line option in `CommandFlags` so `llc/opt` can toggle `TargetOptions::EnableStoreMerging`, and remove the now-redundant DAGCombiner-local `-combiner-store-merging` in favor of this single option. Add tests covering the SelectionDAG path, the GlobalISel path, and the clang driver flag forwarding. This change was developed with the assistance of TraeCLI. --- Full diff: https://github.com/llvm/llvm-project/pull/204081.diff 13 Files Affected: - (modified) clang/include/clang/Basic/CodeGenOptions.def (+1) - (modified) clang/include/clang/Options/Options.td (+5) - (modified) clang/lib/CodeGen/BackendUtil.cpp (+1) - (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3) - (added) clang/test/Driver/fstore-merging.c (+9) - (modified) llvm/include/llvm/CodeGen/CommandFlags.h (+2) - (modified) llvm/include/llvm/Target/TargetOptions.h (+7-1) - (modified) llvm/lib/CodeGen/CommandFlags.cpp (+8) - (modified) llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp (+6) - (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+2-6) - (added) llvm/test/CodeGen/AArch64/GlobalISel/store-merging-disable.ll (+25) - (added) llvm/test/CodeGen/AArch64/store-merging-disable.ll (+25) - (modified) llvm/test/CodeGen/Hexagon/store-widen-aliased-load.ll (+1-1) ``````````diff diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 5f3baf771ff96..2e3994af733de 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -187,6 +187,7 @@ CODEGENOPT(IncrementalLinkerCompatible, 1, 0, Benign) ///< Emit an object file w ///< be used with an incremental ///< linker. CODEGENOPT(MergeAllConstants , 1, 1, Benign) ///< Merge identical constants. +CODEGENOPT(StoreMerging , 1, 1, Benign) ///< Set when store merging is enabled (-fstore-merging). CODEGENOPT(MergeFunctions , 1, 0, Benign) ///< Set when -fmerge-functions is enabled. CODEGENOPT(NoCommon , 1, 0, Benign) ///< Set when -fno-common or C++ is enabled. CODEGENOPT(NoExecStack , 1, 0, Benign) ///< Set when -Wa,--noexecstack is enabled. diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index a4b9cb802af4d..a19bedcd8609d 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -3459,6 +3459,11 @@ defm merge_all_constants : BoolFOption<"merge-all-constants", PosFlag<SetTrue, [], [ClangOption, CC1Option, CLOption], "Allow">, NegFlag<SetFalse, [], [ClangOption], "Disallow">, BothFlags<[], [ClangOption], " merging of constants">>; +defm store_merging : BoolFOption<"store-merging", + CodeGenOpts<"StoreMerging">, DefaultTrue, + PosFlag<SetTrue, [], [ClangOption], "Enable">, + NegFlag<SetFalse, [], [ClangOption, CC1Option], "Disable">, + BothFlags<[], [ClangOption], " merging of adjacent stores into a wider store">>; def fmessage_length_EQ : Joined<["-"], "fmessage-length=">, Group<f_Group>, Visibility<[ClangOption, CC1Option]>, HelpText<"Format message diagnostics so that they fit within N columns">, diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index a46a25c4492f2..f7a3f6cd720a1 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -451,6 +451,7 @@ static bool initTargetOptions(const CompilerInstance &CI, CodeGenOpts.PartitionStaticDataSections; Options.FunctionSections = CodeGenOpts.FunctionSections; Options.DataSections = CodeGenOpts.DataSections; + Options.EnableStoreMerging = CodeGenOpts.StoreMerging; Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility; Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames; Options.UniqueBasicBlockSectionNames = diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 312e8f8c69f0a..b52f724e0a7d8 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5842,6 +5842,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, options::OPT_fno_merge_all_constants, false)) CmdArgs.push_back("-fmerge-all-constants"); + Args.addOptOutFlag(CmdArgs, options::OPT_fstore_merging, + options::OPT_fno_store_merging); + Args.addOptOutFlag(CmdArgs, options::OPT_fdelete_null_pointer_checks, options::OPT_fno_delete_null_pointer_checks); diff --git a/clang/test/Driver/fstore-merging.c b/clang/test/Driver/fstore-merging.c new file mode 100644 index 0000000000000..67acd605609d8 --- /dev/null +++ b/clang/test/Driver/fstore-merging.c @@ -0,0 +1,9 @@ +// Check that -fno-store-merging is forwarded to -cc1 and that the default +// (or explicit -fstore-merging) does not pass the flag. + +// RUN: %clang -### -c %s -fstore-merging -fno-store-merging 2>&1 | FileCheck %s +// CHECK: "-fno-store-merging" + +// RUN: %clang -### -c %s 2>&1 | FileCheck --check-prefix=DEFAULT %s +// RUN: %clang -### -c %s -fno-store-merging -fstore-merging 2>&1 | FileCheck --check-prefix=DEFAULT %s +// DEFAULT-NOT: "-fno-store-merging" diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h index 0af5aa64b29fd..3965a68100aa5 100644 --- a/llvm/include/llvm/CodeGen/CommandFlags.h +++ b/llvm/include/llvm/CodeGen/CommandFlags.h @@ -83,6 +83,8 @@ LLVM_ABI bool getDisableTailCalls(); LLVM_ABI bool getStackSymbolOrdering(); +LLVM_ABI bool getStoreMerging(); + LLVM_ABI bool getStackRealign(); LLVM_ABI std::string getTrapFuncName(); diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h index 4b589926615fd..52dd0f855d1af 100644 --- a/llvm/include/llvm/Target/TargetOptions.h +++ b/llvm/include/llvm/Target/TargetOptions.h @@ -122,7 +122,8 @@ class TargetOptions { : NoTrappingFPMath(true), EnableAIXExtendedAltivecABI(false), HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false), GuaranteedTailCallOpt(false), StackSymbolOrdering(true), - EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false), + EnableStoreMerging(true), EnableFastISel(false), + EnableGlobalISel(false), UseInitArray(false), DisableIntegratedAS(false), FunctionSections(false), DataSections(false), IgnoreXCOFFVisibility(false), XCOFFTracebackTable(true), UniqueSectionNames(true), @@ -194,6 +195,11 @@ class TargetOptions { /// they were generated. Default is true. unsigned StackSymbolOrdering : 1; + /// EnableStoreMerging - This flag enables merging of multiple adjacent + /// stores into a single wider store in the code generator. Default is true. + /// It can be disabled (e.g. via clang's -fno-store-merging). + unsigned EnableStoreMerging : 1; + /// EnableFastISel - This flag enables fast-path instruction selection /// which trades away generated code quality in favor of reducing /// compile time. diff --git a/llvm/lib/CodeGen/CommandFlags.cpp b/llvm/lib/CodeGen/CommandFlags.cpp index e7b9b6e194752..e4491677650b2 100644 --- a/llvm/lib/CodeGen/CommandFlags.cpp +++ b/llvm/lib/CodeGen/CommandFlags.cpp @@ -87,6 +87,7 @@ CGOPT(bool, DontPlaceZerosInBSS) CGOPT(bool, EnableGuaranteedTailCallOpt) CGOPT(bool, DisableTailCalls) CGOPT(bool, StackSymbolOrdering) +CGOPT(bool, StoreMerging) CGOPT(bool, StackRealign) CGOPT(std::string, TrapFuncName) CGOPT(bool, UseCtors) @@ -330,6 +331,12 @@ codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() { cl::init(true)); CGBINDOPT(StackSymbolOrdering); + static cl::opt<bool> StoreMerging( + "store-merging", + cl::desc("Enable merging of adjacent stores into a wider store."), + cl::init(true)); + CGBINDOPT(StoreMerging); + static cl::opt<bool> StackRealign( "stackrealign", cl::desc("Force align the stack to the minimum alignment"), @@ -594,6 +601,7 @@ codegen::InitTargetOptionsFromCodeGenFlags(const Triple &TheTriple) { Options.NoZerosInBSS = getDontPlaceZerosInBSS(); Options.GuaranteedTailCallOpt = getEnableGuaranteedTailCallOpt(); Options.StackSymbolOrdering = getStackSymbolOrdering(); + Options.EnableStoreMerging = getStoreMerging(); Options.UseInitArray = !getUseCtors(); Options.DisableIntegratedAS = getDisableIntegratedAS(); Options.DataSections = diff --git a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp index 365a927a4612f..76b24bac5681c 100644 --- a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp +++ b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp @@ -35,6 +35,7 @@ #include "llvm/Support/Casting.h" #include "llvm/Support/Debug.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Target/TargetMachine.h" #include <algorithm> #define DEBUG_TYPE "loadstore-opt" @@ -919,6 +920,11 @@ bool LoadStoreOpt::mergeTruncStoresBlock(MachineBasicBlock &BB) { } bool LoadStoreOpt::mergeFunctionStores(MachineFunction &MF) { + // Respect the target option for disabling store merging (e.g. clang's + // -fno-store-merging). + if (!MF.getTarget().Options.EnableStoreMerging) + return false; + bool Changed = false; for (auto &BB : MF){ Changed |= mergeBlockStores(BB); diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 4953a9add0df6..c0b0e33df90a8 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -124,11 +124,6 @@ static cl::opt<bool> MaySplitLoadIndex("combiner-split-load-index", cl::Hidden, cl::init(true), cl::desc("DAG combiner may split indexing from loads")); -static cl::opt<bool> - EnableStoreMerging("combiner-store-merging", cl::Hidden, cl::init(true), - cl::desc("DAG combiner enable merging multiple stores " - "into a wider store")); - static cl::opt<unsigned> TokenFactorInlineLimit( "combiner-tokenfactor-inline-limit", cl::Hidden, cl::init(2048), cl::desc("Limit the number of operands to inline for Token Factors")); @@ -23646,7 +23641,8 @@ bool DAGCombiner::tryStoreMergeOfLoads(SmallVectorImpl<MemOpLink> &StoreNodes, } bool DAGCombiner::mergeConsecutiveStores(StoreSDNode *St) { - if (OptLevel == CodeGenOptLevel::None || !EnableStoreMerging) + if (OptLevel == CodeGenOptLevel::None || + !DAG.getTarget().Options.EnableStoreMerging) return false; // TODO: Extend this function to merge stores of scalable vectors. diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/store-merging-disable.ll b/llvm/test/CodeGen/AArch64/GlobalISel/store-merging-disable.ll new file mode 100644 index 0000000000000..b794563349cfb --- /dev/null +++ b/llvm/test/CodeGen/AArch64/GlobalISel/store-merging-disable.ll @@ -0,0 +1,25 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; Check that -store-merging=false disables GlobalISel store merging. +; RUN: llc -mtriple=aarch64-apple-ios -global-isel -global-isel-abort=1 < %s | FileCheck %s --check-prefix=MERGE +; RUN: llc -mtriple=aarch64-apple-ios -global-isel -global-isel-abort=1 -store-merging=false < %s | FileCheck %s --check-prefix=NOMERGE + +define void @test_2xs16(ptr %ptr) { +; MERGE-LABEL: test_2xs16: +; MERGE: ; %bb.0: +; MERGE-NEXT: mov w8, #4 ; =0x4 +; MERGE-NEXT: movk w8, #5, lsl #16 +; MERGE-NEXT: str w8, [x0] +; MERGE-NEXT: ret +; +; NOMERGE-LABEL: test_2xs16: +; NOMERGE: ; %bb.0: +; NOMERGE-NEXT: mov w8, #4 ; =0x4 +; NOMERGE-NEXT: mov w9, #5 ; =0x5 +; NOMERGE-NEXT: strh w8, [x0] +; NOMERGE-NEXT: strh w9, [x0, #2] +; NOMERGE-NEXT: ret + store i16 4, ptr %ptr + %addr2 = getelementptr i16, ptr %ptr, i64 1 + store i16 5, ptr %addr2 + ret void +} diff --git a/llvm/test/CodeGen/AArch64/store-merging-disable.ll b/llvm/test/CodeGen/AArch64/store-merging-disable.ll new file mode 100644 index 0000000000000..e6dea7b0c36c7 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/store-merging-disable.ll @@ -0,0 +1,25 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; Check that -store-merging=false disables SelectionDAG store merging. +; RUN: llc -mtriple=aarch64-apple-ios < %s | FileCheck %s --check-prefix=MERGE +; RUN: llc -mtriple=aarch64-apple-ios -store-merging=false < %s | FileCheck %s --check-prefix=NOMERGE + +define void @test_2xs16(ptr %ptr) { +; MERGE-LABEL: test_2xs16: +; MERGE: ; %bb.0: +; MERGE-NEXT: mov w8, #4 ; =0x4 +; MERGE-NEXT: movk w8, #5, lsl #16 +; MERGE-NEXT: str w8, [x0] +; MERGE-NEXT: ret +; +; NOMERGE-LABEL: test_2xs16: +; NOMERGE: ; %bb.0: +; NOMERGE-NEXT: mov w8, #4 ; =0x4 +; NOMERGE-NEXT: mov w9, #5 ; =0x5 +; NOMERGE-NEXT: strh w8, [x0] +; NOMERGE-NEXT: strh w9, [x0, #2] +; NOMERGE-NEXT: ret + store i16 4, ptr %ptr + %addr2 = getelementptr i16, ptr %ptr, i64 1 + store i16 5, ptr %addr2 + ret void +} diff --git a/llvm/test/CodeGen/Hexagon/store-widen-aliased-load.ll b/llvm/test/CodeGen/Hexagon/store-widen-aliased-load.ll index b614096447eb4..c46ddbff9a10c 100644 --- a/llvm/test/CodeGen/Hexagon/store-widen-aliased-load.ll +++ b/llvm/test/CodeGen/Hexagon/store-widen-aliased-load.ll @@ -1,4 +1,4 @@ -; RUN: llc -mtriple=hexagon --combiner-store-merging=false -verify-machineinstrs < %s | FileCheck %s +; RUN: llc -mtriple=hexagon -store-merging=false -verify-machineinstrs < %s | FileCheck %s ; CHECK: memh ; Check that store widening merges the two adjacent stores. `````````` </details> https://github.com/llvm/llvm-project/pull/204081 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
