[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