[PATCH] D155734: [X86][AArch64] Add additional extract_lowbits test

Danila Malyutin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 19 14:04:29 PDT 2023


danilaml added a comment.

In D155734#4516168 <https://reviews.llvm.org/D155734#4516168>, @goldstein.w.n wrote:

> Your change is x86 only. Why are you adding aarch64 tests?

My change is x86 only, but the tests are not. These tests were added on your suggestion that the coverage for masked `nbits` was lacking, which is not related to the change I was making. And the test file explicitly asks to keep the tests in sync between x86 and aarch64. Do you see any benefit in splitting it further?

> Also you have the order flipped around. The test commit should be the child of the implementation, not the other way around.

Is it not? I don't understand then how this works. Phabricator states that D155622 <https://reviews.llvm.org/D155622> was added as a parent, doesn't that make this (test) commit its child?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155734



More information about the llvm-commits mailing list