[PATCH] D35516: [GISel]: Split buildSources part of buildInstr separately
    Daniel Sanders via Phabricator via llvm-commits 
    llvm-commits at lists.llvm.org
       
    Mon Jul 24 12:29:30 PDT 2017
    
    
  
dsanders added a comment.
In https://reviews.llvm.org/D35516#819039, @aditya_nandakumar wrote:
> @dsanders - the main goal was to avoid the compiler warning of having to create a zero sized array.
You mean the `unsigned It[] = {};`? Ok, that makes sense. The subject line should reflect that though. E.g:
  [GISel] Avoid zero-length array when adding uses to instructions that lack them (e.g IMPLICIT_DEF).
================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:77-80
+  void addUseFromArg(MachineInstrBuilder &MIB, const MachineOperand &MO) {
+    MIB.add(MO);
+  }
 
----------------
This should be added by the patch that uses it.
================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:85
 
+  void buildSources(MachineInstrBuilder &MIB) { }
+  template<typename UseArgTy, typename ... UseArgsTy>
----------------
Just for consistency, should we name this addUsesFromArgs()?
================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:88-89
+  void buildSources(MachineInstrBuilder &MIB, UseArgTy &&Arg1, UseArgsTy &&... Args) {
+    addUseFromArg(MIB, Arg1);
+    buildSources(Args...);
+  }
----------------
I think this is missing some std::forward's
https://reviews.llvm.org/D35516
    
    
More information about the llvm-commits
mailing list