[PATCH] D102766: [SelectionDAG] Implement PromoteIntRes_INSERT_SUBVECTOR

Thu May 20 10:42:29 PDT 2021

paulwalker-arm added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4704
+SDValue DAGTypeLegalizer::PromoteIntRes_INSERT_SUBVECTOR(SDNode *N) {
+
+  EVT OutVT = N->getValueType(0);
----------------
peterwaller-arm wrote:
> joechrisellis wrote:
> > peterwaller-arm wrote:
> > > nit: Extraneous blank.
> > This is consistent with `PromoteIntRes_EXTRACT_SUBVECTOR` some 60-ish lines above.
> This is only a nit, but the majority of functions in this file don't have it and I think PromoteIntRes_EXTRACT_SUBVECTOR is in error. It's sufficiently nearby and related code that I'd be tempted to remove it from that one too to maintain local consistenc (but not anywhere else, there are other examples in this file).
Functional issues aside, I'm with @peterwaller-arm on this one.

================
Comment at: llvm/test/CodeGen/AArch64/insert-subvector-res-legalization.ll:14
+; CHECK-NEXT:    .cfi_escape 0x0f, 0x0c, 0x8f, 0x00, 0x11, 0x10, 0x22, 0x11, 0x08, 0x92, 0x2e, 0x00, 0x1e, 0x22 // sp + 16 + 8 * VG
+; CHECK-NEXT:    .cfi_offset w29, -16
+; CHECK-NEXT:    ptrue p0.h
----------------
joechrisellis wrote:
> peterwaller-arm wrote:
> > I think you can use -asm-verbose=0 in the run line to eliminate the CFI escapes.
> I tried this and `llvm/utils/update_llc_test_checks.py` doesn't spit anything out. I have hit this issue before. IIRC, the regular expression that is used by the the script to delimit the functions in the assembly codes doesn't function as expected if the CFI escapes are missing. Might submit a patch for this later if I can recall what the issue was.
> 
> FWIW:
> 
> ```
> $ grep -Rl 'Assertions have been' llvm/test/**/AArch64/**/* | xargs grep -l 'asm-verbose=0'
> llvm/test/CodeGen/AArch64/bf16-vector-shuffle.ll
> ```
> 
> ... there's only one file with autogen'd assertions that does use `-asm-verbose=0`.
`attributes #0 = { nounwind "target-features"="+sve" }` will see the CFI entries removed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102766