[PATCH] D143361: [RISCV] Support __builtin_nontemporal_load/store by MachineMemOperand

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

The way other targets appear to be approach this is to modify ISEL.  Can you explain why you believe the late hint insertion pass is the right approach here?  The alternative would be to have a family of non-temporal pseudos which expanded into a hint + a memory op.  I don't have a strong opinion, just curious for the justification.



================
Comment at: llvm/test/CodeGen/RISCV/nontemporal-c.ll:5
+
+define i64 @test_nontemporal_load_i64(i64* %p) {
+; CHECK-RV64-LABEL: test_nontemporal_load_i64:
----------------
These need updated to use modern "ptr" syntax.  


================
Comment at: llvm/test/CodeGen/RISCV/nontemporal-vector.ll:42
+
+define i16 @test_nontemporal_load_i16(i16* %p) {
+; CHECK-RV64-LABEL: test_nontemporal_load_i16:
----------------
Having scalar tests in a vector file is a bit odd...


================
Comment at: llvm/test/CodeGen/RISCV/nontemporal.ll:465
+
+  %1 = load <2 x i64>, <2 x i64>* %p, align 16, !nontemporal !0
+  ret <2 x i64> %1
----------------
I couple of general issues with your tests:
* It's not really clear what the separation between the test files is.  You appear to repeat tests multiple times.  Did you maybe want to use multiple check lines in a single file?
* You don't appear to have any test coverage of scalable vectors.
* You need to update syntax to use modern "ptr".
* You mix volatile and non-volatile without discussion on why (that I saw on a quick skim).  In general, you should not be using volatile you're actually testing volatile.  (i.e. don't use it just to "stabilize a test")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143361