rampitec added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12120
+                   OptimizationRemarkEmitter *ORE,
+                   OptimizationRemark OptRemark) {
+  ORE->emit([&]() { return OptRemark; });
----------------
gandhi21299 wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > gandhi21299 wrote:
> > > > rampitec wrote:
> > > > > gandhi21299 wrote:
> > > > > > rampitec wrote:
> > > > > > > Why OptRemark and not just StringRef? I really want to see as 
> > > > > > > little churn as possible at the call site.
> > > > > > With only StringRef, we would also have to pass in RMW since 
> > > > > > OptimizationRemark constructor depends on that.
> > > > > It seems better be a lamda where you can just capture most of the 
> > > > > stuff.
> > > > What would the type be for the following lambda?
> > > > 
> > > > ```
> > > > [&](){
> > > >     return OptimizationRemark(...);
> > > > }
> > > > ```
> > > You need to return TargetLowering::AtomicExpansionKind, not 
> > > OptimizationRemark.
> > This is what goes into ORE->emit() as an argument though. I suspect the 
> > remark won't be emitted if I return AtomicExpansionKind within the lambda.
> @rampitec I don't think there is a way to pass the ORE->emit argument lambda 
> expression into the reportAtomicExpand() function because of its capture. It 
> needs access to RMW from the enclosing scope. (Well, I could define a copy of 
> RMW and somehow pass it into the lambda but that seems too much for this task)
Do not pass it there. Turn reportAtomicExpand into lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to