[PATCH] Add a RequireStructuredCFG Field to TargetMachine.

[Tom Stellard]

Hi Vincent,

This patch LGTM.  cc'ing other potential reviewers.

-Tom

On Mon, Nov 25, 2013 at 08:35:44AM -0800, Vincent Lejeune wrote:
>   Thank for the review, I updated the patch.
> 
> http://llvm-reviews.chandlerc.com/D2260
> 
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D2260?vs=5750&id=5766#toc
> 
> Files:
>   docs/WritingAnLLVMBackend.rst
>   include/llvm/Target/TargetMachine.h
>   lib/CodeGen/BranchFolding.cpp
>   lib/CodeGen/MachineBasicBlock.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
> @@ -87,6 +87,7 @@
>    unsigned MCUseLoc : 1;
>    unsigned MCUseCFI : 1;
>    unsigned MCUseDwarfDirectory : 1;
> +  unsigned RequireStructuredCFG : 1;
>  
>  public:
>    virtual ~TargetMachine();
> @@ -155,6 +156,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/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
> @@ -87,6 +87,7 @@
>    unsigned MCUseLoc : 1;
>    unsigned MCUseCFI : 1;
>    unsigned MCUseDwarfDirectory : 1;
> +  unsigned RequireStructuredCFG : 1;
>  
>  public:
>    virtual ~TargetMachine();
> @@ -155,6 +156,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/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