[PATCH] D15471: [IR] Add support for floating pointer atomic loads and stores

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 15 16:49:00 PST 2015


reames added inline comments.

================
Comment at: test/CodeGen/X86/atomic-non-integer.ll:2
@@ +1,3 @@
+; RUN: llc < %s -mtriple=x86_64-linux-generic -verify-machineinstrs -mattr=sse2 | FileCheck %s
+
+; Note: This test is testing that the lowering for atomics matches what we
----------------
I added a note to the top of the file.  Let me know if you want something more.

================
Comment at: test/CodeGen/X86/atomic-non-integer.ll:7
@@ +6,3 @@
+; that detail for correctness unless it's related to the atomicity itself.
+; (Specifically, there were reviewer questions about the lowering for halfs
+;  and their calling convention which remain unresolved.)
----------------
This comment doesn't really parse for me.  If you want to suggest a change here, please raise it on the mailing lists so that someone more knowledgeable than I can comment.  

================
Comment at: test/Verifier/atomics.ll:4
@@ +3,3 @@
+; CHECK: atomic store operand must have integer or floating point type!
+; CHECK: atomic load operand must have integer or floating point type!
+
----------------
jfb wrote:
> Ha, that's not the best error message since `x86_mmx` is a floating-point type. I'll rework it in D15512 if that's OK with you.
Not according to LLVM's isFloatingPointTy it's not.  Surprised me too.  Fixing that might end up being a much larger change though.


http://reviews.llvm.org/D15471





More information about the llvm-commits mailing list