[PATCH] D84471: [X86] Fix for ballooning compile times due to Load Value Injection (LVI) mitigations

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 27 14:26:40 PDT 2020


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:664
     MachineFunction &MF, std::unique_ptr<MachineGadgetGraph> Graph) const {
-  LLVM_DEBUG(dbgs() << "Eliminating mitigated paths...\n");
-  Graph = trimMitigatedEdges(std::move(Graph));
-  LLVM_DEBUG(dbgs() << "Eliminating mitigated paths... Done\n");
+  using NodeRef = const MachineGadgetGraph::Node *;
+  using EdgeRef = const MachineGadgetGraph::Edge *;
----------------
I'm not a big fan of calling a pointer a "Ref". It makes me think its a reference but you have to dereference it. Do we gain much from omitting the '*' where this is used?


================
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:716
+          IngressCutCost < EgressCutCost ? IngressEdges : EgressEdges;
+      llvm::for_each(EdgesToCut, [&](EdgeRef E) { CutEdges.insert(*E); });
     }
----------------
I think llvm::for_each is discouraged in this case by the statement here https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84471/new/

https://reviews.llvm.org/D84471





More information about the llvm-commits mailing list