[PATCH] D157976: [RISCV] Use materialization cost when lowering constant build_vector

Luke Lau via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 17 10:03:09 PDT 2023


luke added inline comments.
Herald added a subscriber: sunshaoce.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3294
+    if (isPowerOf2_32(StepDenominator) && (SplatStepVal >= 0 || StepDenominator == 1)) {
+      AddLowering(Cost, [=, &DAG, &Subtarget]() {
+        MVT VIDVT =
----------------
reames wrote:
> Copying everything by value here to avoid scope issues is rather subtle.  I don't spot a scope bug here, but I'm not a huge fan of the code structure. 
Not a fan either. I admittedly spent some time trying to track down a miscompile due to UB, where I had originally copied some stack variables by reference.

Two possible alternatives:
- Explicitly specify the list of captures
- Create classes for each type of lowering and move DAG logic & cost logic into them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157976



More information about the llvm-commits mailing list