[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/CodeGen/CGBuiltin.cpp:570
+        llvm::AtomicOrdering::SequentiallyConsistent);
+    llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+    llvm::Value *Truncated =
----------------
hans wrote:
> rnk wrote:
> > Can you comment that this shifts the relevant bit into the low bit, truncates to i8, and then tests the low bit? At first I was thinking "of course, we just do AND %old, $constant to test the bit".
> > 
> > Do we successfully pattern match this idiom to `lock bts`? Is there a pattern match we should target?
> Added the comment.
> 
> No, we generate horrible code for this, but it's the same as we did for the inline version in intrin.h. We have BTS instructions in the .td file, but no patterns for them.
> 
> I thought a bit about adding a pattern match for this, but I'm not sure it would be worth the effort.
Let's file a bug for it and call it a day.


https://reviews.llvm.org/D31736