[PATCH] D118549: apply two more cases for store combine

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7632
+  unsigned PreShift = INT32_MAX;
+  for(auto Iter : OffsetMap) {
+    if(PreShift != INT32_MAX){
----------------
Run clang-format, the formatting for much of your new code is wrong


================
Comment at: llvm/test/CodeGen/RISCV/store-combine.ll:43
+
+define void @store_0(%struct.ab* nocapture noundef writeonly %_ab, i32 noundef %v) {
+; RV32-LABEL: store_0:
----------------
Do we need any of these attributes?


================
Comment at: llvm/test/CodeGen/RISCV/store-combine.ll:139
+; not to combine stores
+; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn writeonly
+define dso_local void @store4(%struct.ab* nocapture noundef writeonly %_ab, i32 noundef %v) local_unnamed_addr #0 {
----------------
This is entirely useless


================
Comment at: llvm/test/CodeGen/RISCV/store-combine.ll:140
+; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn writeonly
+define dso_local void @store4(%struct.ab* nocapture noundef writeonly %_ab, i32 noundef %v) local_unnamed_addr #0 {
+; RV32-LABEL: store4:
----------------
`local_unnamed_addr #0` likely aren't needed? (and `#0` doesn't exist)


================
Comment at: llvm/test/CodeGen/RISCV/store-combine.ll:79
+
+!4 = !{!5, !6, i64 0}
+!5 = !{!"ab", !6, i64 0, !6, i64 1, !6, i64 2, !6, i64 3}
----------------
BaoshanPang wrote:
> frasercrmck wrote:
> > Do we need all of this metadata for the test to work? I presume we don't need TBAA since the stores trivially don't alias.
> I am not sure how to remove TBAA(!7). 
> !7 is referenced by !6, i6 is referenced by all other variables. If I remove !7, it seems I will need to remove all other variables.
But are //any// of them needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118549