[PATCH] D114460: [fir] Add fir reduction builder

Thu Nov 25 07:31:30 PST 2021

clementval added inline comments.

================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:25-26
+  auto loc = firBuilder->getUnknownLoc();
+  mlir::Type seqTy =
+      fir::SequenceType::get(fir::SequenceType::Shape(1, 10), i32Ty);
+  mlir::Value result = firBuilder->create<fir::UndefOp>(loc, seqTy);
----------------
awarzynski wrote:
> clementval wrote:
> > awarzynski wrote:
> > > clementval wrote:
> > > > awarzynski wrote:
> > > > > Move next to other "// Commonly used types" in `RuntimeCallTest`?
> > > > Are you talking about `seqTy` or `result`? Not sure if the comment moved with the update. 
> > > Both are re-defined multiple times. So is `undef`, `dim` and a few other. I'd keep only keep the bare minimum in every test that's unique to a particular case being tested. Reduced code-duplication, makes this file shorter.
> > Values have to be created right here otherwise the test does not make sense. Type can be moved to the SetUp. 
> > Values have to be created right here otherwise the test does not make sense.
> 
> Why not? Do you think that this way it will be clearer or is there a technical reason?
No technical reason but I think it's clearer to create the operations used in the call right here.  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114460