MaskRay added inline comments.
================ Comment at: llvm/test/CodeGen/X86/movdir64b-inline-asm-x86_64.ll:1 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2 +; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+movdir64b | FileCheck %s --check-prefix=X64 ---------------- akshaykhadse wrote: > MaskRay wrote: > > I think we should reuse an existing `*-inline-asm-*` test file. > > > > See > > https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer > > "I don't know an existing test can be enhanced" > **I could not find an appropriate existing test.** > I added a couple of cases in the CFE tests > `clang/test/CodeGen/ms-inline-asm-64.c` and > `clang/test/CodeGen/ms-inline-asm.c`. > Now, when I had to add backend tests, I searched for an appropriate file: > ``` > llvm-project$ find llvm/test/CodeGen/X86 -type f -name ms-inline-asm-* > llvm/test/CodeGen/X86/ms-inline-asm-variables-x86-2-regs.ll > llvm/test/CodeGen/X86/ms-inline-asm-array.ll > llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll > llvm/test/CodeGen/X86/ms-inline-asm-variables-x64-2-regs.ll > llvm/test/CodeGen/X86/ms-inline-asm-functions.ll > llvm/test/CodeGen/X86/ms-inline-asm-redundant-clobber.ll > llvm/test/CodeGen/X86/ms-inline-asm-variables-x64-1-reg.ll > llvm/test/CodeGen/X86/ms-inline-asm-variables-x86-1-reg.ll > llvm/test/CodeGen/X86/ms-inline-asm-avx512.ll > llvm/test/CodeGen/X86/ms-inline-asm-variables-x64-nopic.ll > ``` > As you can see, there are tests for variables, functions, arrays, but no > general tests like `ms-inline-asm.ll`. > My general observation is that for a CFE test `*-inline-asm*.c` there's an > equivalent test in LLVM backend which mentions the original test name in the > comment. For example, `ms-inline-asm-variables-x86-2-regs.ll` mentions that > `Tests come from "clang/test/CodeGen/ms-inline-asm-variables.c"`. > However, there is no such file which mentions either `ms-inline-asm.c` or > `ms-inline-asm-64.c` > So, I picked up an assembly instruction `xgetbv` from `ms-inline-asm.c` and > searched it in the available `.ll` backend test via `llvm-project$ find > llvm/test -type f -name *.ll -exec grep -l "xgetbv" {} \;`. There was just > one file `llvm/test/CodeGen/X86/system-intrinsics-xgetbv.ll` that contains > test for just this one instruction. > **Please suggest an appropriate 32-bit and 64-bit file to add this test** Thanks for the investigation. Sometimes we can consider refactoring existing tests before adding a new test. Even if we add a new file, `inline-asm-*.ll` may be a better name than `*-inline-asm.ll`, since the former allows code completion to find related tests more easily. I'll run away:) My comment is to prompt investigation but I don't plan to look at the problem closely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151863/new/ https://reviews.llvm.org/D151863 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
