[PATCH] D129109: [Costmodel] Add type based costmodel analysis for masked scatter/gather intrinsics

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

A couple of drive-through Nit comments. Feel free to skip.



================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:1787-1793
+      // arguments below are actually not used by
+      // getGatherScatterOpCost
+      const Value *Ptr = nullptr;
+      const Instruction *I = nullptr;
+      return thisT()->getGatherScatterOpCost(Instruction::Store,
+                                             Ty, Ptr, VarMask, TyAlign,
+                                             CostKind, I);
----------------
Nit: Would this be better for here and below?


================
Comment at: llvm/lib/Analysis/CostModel.cpp:118
+      else
+        Cost = TTI->getInstructionCost(&Inst, CostKind);
       if (auto CostVal = Cost.getValue())
----------------
Nit: Consider using braces everywhere in an if-else construct, here and below.

" For example, readability is also harmed if an if/else chain does not use braced bodies for either all or none of its members, ... etc."
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129109