[PATCH] Add a RequireStructuredCFG Field to TargetMachine.
Tom Stellard
tom at stellard.net
Tue Dec 3 15:19:00 PST 2013
Hi Vincent,
This patch LGTM. cc'ing other potential reviewers who may have missed
this over the holiday break.
-Tom
On Mon, Dec 02, 2013 at 12:01:48PM -0800, Vincent Lejeune wrote:
> Now prevents the tailduplication pass.
>
> http://llvm-reviews.chandlerc.com/D2260
>
> CHANGE SINCE LAST DIFF
> http://llvm-reviews.chandlerc.com/D2260?vs=5766&id=5858#toc
>
> Files:
> docs/WritingAnLLVMBackend.rst
> include/llvm/Target/TargetMachine.h
> lib/CodeGen/BranchFolding.cpp
> lib/CodeGen/MachineBasicBlock.cpp
> lib/CodeGen/Passes.cpp
> lib/Target/R600/AMDGPUTargetMachine.cpp
> lib/Target/TargetMachine.cpp
>
> Index: docs/WritingAnLLVMBackend.rst
> ===================================================================
> --- docs/WritingAnLLVMBackend.rst
> +++ docs/WritingAnLLVMBackend.rst
> @@ -238,6 +238,12 @@
> * ``getTargetLowering()``
> * ``getJITInfo()``
>
> +Some architectures, such as GPUs, do not support jumping to an arbitrary
> +program location and implement branching using masked execution and loop using
> +special instructions around the loop body. In order to avoid CFG modifications
> +that introduce irreducible control flow not handled by such hardware, a target
> +must call `setRequiresStructuredCFG(true)` when being initialized.
> +
> In addition, the ``XXXTargetMachine`` constructor should specify a
> ``TargetDescription`` string that determines the data layout for the target
> machine, including characteristics such as pointer size, alignment, and
> Index: include/llvm/Target/TargetMachine.h
> ===================================================================
> --- include/llvm/Target/TargetMachine.h
> +++ include/llvm/Target/TargetMachine.h
> @@ -88,6 +88,7 @@
> unsigned MCUseLoc : 1;
> unsigned MCUseCFI : 1;
> unsigned MCUseDwarfDirectory : 1;
> + unsigned RequireStructuredCFG : 1;
>
> public:
> virtual ~TargetMachine();
> @@ -156,6 +157,9 @@
> return 0;
> }
>
> + bool requiresStructuredCFG() const { return RequireStructuredCFG; }
> + void setRequiresStructuredCFG(bool Value) { RequireStructuredCFG = Value; }
> +
> /// hasMCRelaxAll - Check whether all machine code instructions should be
> /// relaxed.
> bool hasMCRelaxAll() const { return MCRelaxAll; }
> Index: lib/CodeGen/BranchFolding.cpp
> ===================================================================
> --- lib/CodeGen/BranchFolding.cpp
> +++ lib/CodeGen/BranchFolding.cpp
> @@ -83,7 +83,11 @@
>
> bool BranchFolderPass::runOnMachineFunction(MachineFunction &MF) {
> TargetPassConfig *PassConfig = &getAnalysis<TargetPassConfig>();
> - BranchFolder Folder(PassConfig->getEnableTailMerge(), /*CommonHoist=*/true);
> + // TailMerge can create jump into if branches that make CFG irreducible for
> + // HW that requires structurized CFG.
> + bool EnableTailMerge = !MF.getTarget().requiresStructuredCFG() &&
> + PassConfig->getEnableTailMerge();
> + BranchFolder Folder(EnableTailMerge, /*CommonHoist=*/true);
> return Folder.OptimizeFunction(MF,
> MF.getTarget().getInstrInfo(),
> MF.getTarget().getRegisterInfo(),
> Index: lib/CodeGen/MachineBasicBlock.cpp
> ===================================================================
> --- lib/CodeGen/MachineBasicBlock.cpp
> +++ lib/CodeGen/MachineBasicBlock.cpp
> @@ -677,6 +677,11 @@
> MachineFunction *MF = getParent();
> DebugLoc dl; // FIXME: this is nowhere
>
> + // Performances might be harmed by HW implementing branching using exec mask
> + // and always executes both side of the branches anyway.
> + if (MF->getTarget().requiresStructuredCFG())
> + return NULL;
> +
> // We may need to update this's terminator, but we can't do that if
> // AnalyzeBranch fails. If this uses a jump table, we won't touch it.
> const TargetInstrInfo *TII = MF->getTarget().getInstrInfo();
> Index: lib/CodeGen/Passes.cpp
> ===================================================================
> --- lib/CodeGen/Passes.cpp
> +++ lib/CodeGen/Passes.cpp
> @@ -725,7 +725,10 @@
> printAndVerify("After BranchFolding");
>
> // Tail duplication.
> - if (addPass(&TailDuplicateID))
> + // Note that duplicating tail just increase code size and only degrades
> + // performance in case of target that requires Structured Control Flow.
> + // In addition it can also make CFG irreducible. Thus we disable it.
> + if (!TM->requiresStructuredCFG() && addPass(&TailDuplicateID))
> printAndVerify("After TailDuplicate");
>
> // Copy propagation.
> Index: lib/Target/R600/AMDGPUTargetMachine.cpp
> ===================================================================
> --- lib/Target/R600/AMDGPUTargetMachine.cpp
> +++ lib/Target/R600/AMDGPUTargetMachine.cpp
> @@ -72,6 +72,7 @@
> InstrInfo.reset(new SIInstrInfo(*this));
> TLInfo.reset(new SITargetLowering(*this));
> }
> + setRequiresStructuredCFG(true);
> initAsmInfo();
> }
>
> Index: lib/Target/TargetMachine.cpp
> ===================================================================
> --- lib/Target/TargetMachine.cpp
> +++ lib/Target/TargetMachine.cpp
> @@ -55,6 +55,7 @@
> MCUseLoc(true),
> MCUseCFI(true),
> MCUseDwarfDirectory(false),
> + RequireStructuredCFG(false),
> Options(Options) {
> }
> Index: docs/WritingAnLLVMBackend.rst
> ===================================================================
> --- docs/WritingAnLLVMBackend.rst
> +++ docs/WritingAnLLVMBackend.rst
> @@ -238,6 +238,12 @@
> * ``getTargetLowering()``
> * ``getJITInfo()``
>
> +Some architectures, such as GPUs, do not support jumping to an arbitrary
> +program location and implement branching using masked execution and loop using
> +special instructions around the loop body. In order to avoid CFG modifications
> +that introduce irreducible control flow not handled by such hardware, a target
> +must call `setRequiresStructuredCFG(true)` when being initialized.
> +
> In addition, the ``XXXTargetMachine`` constructor should specify a
> ``TargetDescription`` string that determines the data layout for the target
> machine, including characteristics such as pointer size, alignment, and
> Index: include/llvm/Target/TargetMachine.h
> ===================================================================
> --- include/llvm/Target/TargetMachine.h
> +++ include/llvm/Target/TargetMachine.h
> @@ -88,6 +88,7 @@
> unsigned MCUseLoc : 1;
> unsigned MCUseCFI : 1;
> unsigned MCUseDwarfDirectory : 1;
> + unsigned RequireStructuredCFG : 1;
>
> public:
> virtual ~TargetMachine();
> @@ -156,6 +157,9 @@
> return 0;
> }
>
> + bool requiresStructuredCFG() const { return RequireStructuredCFG; }
> + void setRequiresStructuredCFG(bool Value) { RequireStructuredCFG = Value; }
> +
> /// hasMCRelaxAll - Check whether all machine code instructions should be
> /// relaxed.
> bool hasMCRelaxAll() const { return MCRelaxAll; }
> Index: lib/CodeGen/BranchFolding.cpp
> ===================================================================
> --- lib/CodeGen/BranchFolding.cpp
> +++ lib/CodeGen/BranchFolding.cpp
> @@ -83,7 +83,11 @@
>
> bool BranchFolderPass::runOnMachineFunction(MachineFunction &MF) {
> TargetPassConfig *PassConfig = &getAnalysis<TargetPassConfig>();
> - BranchFolder Folder(PassConfig->getEnableTailMerge(), /*CommonHoist=*/true);
> + // TailMerge can create jump into if branches that make CFG irreducible for
> + // HW that requires structurized CFG.
> + bool EnableTailMerge = !MF.getTarget().requiresStructuredCFG() &&
> + PassConfig->getEnableTailMerge();
> + BranchFolder Folder(EnableTailMerge, /*CommonHoist=*/true);
> return Folder.OptimizeFunction(MF,
> MF.getTarget().getInstrInfo(),
> MF.getTarget().getRegisterInfo(),
> Index: lib/CodeGen/MachineBasicBlock.cpp
> ===================================================================
> --- lib/CodeGen/MachineBasicBlock.cpp
> +++ lib/CodeGen/MachineBasicBlock.cpp
> @@ -677,6 +677,11 @@
> MachineFunction *MF = getParent();
> DebugLoc dl; // FIXME: this is nowhere
>
> + // Performances might be harmed by HW implementing branching using exec mask
> + // and always executes both side of the branches anyway.
> + if (MF->getTarget().requiresStructuredCFG())
> + return NULL;
> +
> // We may need to update this's terminator, but we can't do that if
> // AnalyzeBranch fails. If this uses a jump table, we won't touch it.
> const TargetInstrInfo *TII = MF->getTarget().getInstrInfo();
> Index: lib/CodeGen/Passes.cpp
> ===================================================================
> --- lib/CodeGen/Passes.cpp
> +++ lib/CodeGen/Passes.cpp
> @@ -725,7 +725,10 @@
> printAndVerify("After BranchFolding");
>
> // Tail duplication.
> - if (addPass(&TailDuplicateID))
> + // Note that duplicating tail just increase code size and only degrades
> + // performance in case of target that requires Structured Control Flow.
> + // In addition it can also make CFG irreducible. Thus we disable it.
> + if (!TM->requiresStructuredCFG() && addPass(&TailDuplicateID))
> printAndVerify("After TailDuplicate");
>
> // Copy propagation.
> Index: lib/Target/R600/AMDGPUTargetMachine.cpp
> ===================================================================
> --- lib/Target/R600/AMDGPUTargetMachine.cpp
> +++ lib/Target/R600/AMDGPUTargetMachine.cpp
> @@ -72,6 +72,7 @@
> InstrInfo.reset(new SIInstrInfo(*this));
> TLInfo.reset(new SITargetLowering(*this));
> }
> + setRequiresStructuredCFG(true);
> initAsmInfo();
> }
>
> Index: lib/Target/TargetMachine.cpp
> ===================================================================
> --- lib/Target/TargetMachine.cpp
> +++ lib/Target/TargetMachine.cpp
> @@ -55,6 +55,7 @@
> MCUseLoc(true),
> MCUseCFI(true),
> MCUseDwarfDirectory(false),
> + RequireStructuredCFG(false),
> Options(Options) {
> }
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list