[PATCH] D60294: [DAGCombiner] [CodeGenPrepare] Split large offsets from base addresses

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4203
              !isa<GetElementPtrInst>(BaseI))) {
           // If the base is an instruction, make sure the GEP is not in the same
           // basic block as the base. If the base is an argument or global
----------------
This comment is now out-of-date, and needs updating.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1040
+  for (SDNode *Node : N0->uses()) {
+    auto *LD = dyn_cast<LoadSDNode>(Node);
+    auto *ST = dyn_cast<StoreSDNode>(Node);
----------------
Can you not check for a MemSDNode here, and avoid worrying about whether it's a LD or a ST?


================
Comment at: test/CodeGen/RISCV/split-offsets-1.ll:3
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32I
+
----------------
Should probably check RV64 too


================
Comment at: test/CodeGen/RISCV/split-offsets-1.ll:4
+; RUN:   | FileCheck %s -check-prefix=RV32I
+
+define void @test1([65536 x i32]** %sp, [65536 x i32]* %t, i32 %n) {
----------------
A comment should explain what this test is intending to check


================
Comment at: test/CodeGen/RISCV/split-offsets-2.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
----------------
This doesn't need to be in a separate file to the other test


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60294