[PATCH] D51969: [AMDGPU] Add an AMDGPU specific atomic optimizer.

[Neil Henning via Phabricator via llvm-commits]

sheredom added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp:243-244
+  Value *const ExtractHi = B.CreateExtractElement(BitCast, B.getInt32(1));
+  Function *const MbcntLoIntrinsic =
+      Intrinsic::getDeclaration(I.getModule(), Intrinsic::amdgcn_mbcnt_lo);
+  Function *const MbcntHiIntrinsic =
----------------
nhaehnle wrote:
> sheredom wrote:
> > arsenm wrote:
> > > CreateIntrinsic
> > I can't use the IRBuilder's CreateIntrinsic because it will then try and mangle the amdgcn_mbcnt_lo like llvm.amdgcn.mbcnt.lo.i32, whereas we expect the name to be sans-type-suffix.
> Ugh, this is a bug in CreateIntrinsic. It does:
> ```
>   Function *Fn = Intrinsic::getDeclaration(M, ID, { Args.front()->getType() });
> ```
> ... when in this particular case, the type vector should be empty.
> 
> There aren't that many users of CreateIntrinsic -- can you just fix CreateIntrinsic so that it takes an `ArrayRef<Type*>` like Intrinsic::getDeclaration? (As a separate preparatory patch obviously.)
Just to be clear - do you want me to get the IRBuilder fix in first? Or is it fine to let this patch in as is, and then fixup during the IRBuilder change?


https://reviews.llvm.org/D51969