[PATCH] D67046: [RISCV] Add InstrInfo areMemAccessesTriviallyDisjoint hook

[Luís Marques via Phabricator via llvm-commits]

luismarques marked 2 inline comments as done.
luismarques added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/disjoint.ll:27
 ; CHECK-NEXT:    lb a5, 0(a2)
+; CHECK-NEXT:    sb a3, 1(a2)
 ; CHECK-NEXT:    addi a5, a5, 1
----------------
lenary wrote:
> This test seems really brittle. 
> 
> Now that you've implemented the hook, the compiler can see that this load and store do not interact, so can now schedule the store after the load. However, what the hook actually means for these instructions is that the load and store don't interact, and so any more-specific schedule could undo this change by implementing a schedule that thinks if you do the store first, the code is faster.
> 
> I don't know how to write a test that's less brittle to this kind of issue. I suppose a lot of asm tests could be changed by a change in the schedule, but it seems like this one has a large chance of suddenly not testing anything. 
> 
> Do you have any better ideas for how to test this, or do we not have enough testcases that hit the hook and are trivially disjoint?
You're right, this merited some context and explanation. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67046