[PATCH] D77352: [llvm] Fix missing FileCheck directive colons

Jonathan Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 3 08:02:39 PDT 2020


jroelofs marked 4 inline comments as done.
jroelofs added a comment.

I suppose I should have separated out the trivial ones from these that took a bit more reasoning to confirm.



================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:46
   ret float %v
-; CHECK-LABEL @test_fmul
 }
----------------
jhenderson wrote:
> I'm not sure I follow why it's safe to remove the lines from this test?
Have a look at the patch that added them in the first place: d52990c71b6f83c9beeba4efc9103412e0416ba9

They can't have been correct then, since PPC doesn't use `@`s as a label prefix, but they were pretty clearly intended to mark the beginning of the function. Later on in a51c61ea332f89dfbb9f3b3498c37b2efc99e13b, the test got reworked with `utils/update_llc_test_checks.py`, which didn't know to delete them.


================
Comment at: llvm/test/CodeGen/PowerPC/umulo-128-legalisation-lowering.ll:154
 ; PPC32-NEXT:    blr
-; PPC32-LABEL muloti_test:
 start:
----------------
jhenderson wrote:
> Are you sure this line should be removed?
That label can't appear twice in the ASM, and it's already matched above on line 36.


================
Comment at: llvm/test/CodeGen/X86/GlobalISel/irtranslator-callingconv.ll:318
 define i32 * @test_memop_i32(i32 * %p1) {
-;X64    liveins: $rdi
   ; X32-LABEL: name: test_memop_i32
----------------
jhenderson wrote:
> Are you sure this line should be removed?
This one came from a8ba572dcfdb969fe1bb3381663298488891f838, and the subsequent rewrite in 347921a28157daf87acfcb31ce4787a46bd7ad12 is what left it in the wrong place breaking it further.


================
Comment at: llvm/test/CodeGen/X86/avx-vzeroupper.ll:86
 ; BTVER2-NEXT:    retq
-; DISABLE-VZ       # %bb.0:
   %tmp = load <4 x float>, <4 x float>* @x, align 16
----------------
jhenderson wrote:
> Any thoughts on why this line is here, and why it is safe to remove (i.e. instead of fixing)?
This one wasn't correct when it got added in b2b6a54f847f33f821f41e3e82bf3b86e08817a0, as that block label was already matched by another check a few lines earlier in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77352





More information about the llvm-commits mailing list