[PATCH] D140965: [GlobalISel] Add G_BUILD_VECTOR[_TRUNC] to CSE

Thu Jan 5 01:29:38 PST 2023

rovka added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-mulo-zero.mir:91
+    ; CHECK-NEXT: %zero_vec:_(<2 x s64>) = G_BUILD_VECTOR %zero(s64), %zero(s64)
+    ; CHECK-NEXT: %mulo:_(<2 x s64>) = COPY %zero_vec(<2 x s64>)
     ; CHECK-NEXT: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 false
----------------
foad wrote:
> What happened here?
Well, my hypothesis is that:
Without CSE for G_BUILD_VECTOR, we replace the G_UMULO with a brand new G_BUILD_VECTOR, thus leaving the already existing G_BUILD_VECTOR (%zero_vec) without a use, so it just gets removed.
With CSE enabled, when we try to build the new G_BUILD_VECTOR for our combine, we notice we already have one and just copy it, so now it has a use and can't be dropped. I'm not sure why we don't notice that we can use it directly instead of copying though.
I could dig deeper into this, unless we have some code elsewhere in the pipeline that gets rid of redundant copies?

================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-build-vector-splat.mir:1
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1100 -run-pass=legalizer %s -o - | FileCheck %s
----------------
foad wrote:
> Maybe precommit this test so we can see the diff?
[[ https://github.com/llvm/llvm-project/commit/61c5775b36ebedca0deb0fe59c46ecc7cfcbf209 | Sure ]]

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140965