[PATCH] D122003: [Triple] Add support for more archs in `getArchTypeForLLVMName`
Renato Golin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 22 03:27:59 PDT 2022
rengolin added a comment.
This is already tested by its users (via `lookupTarget`) in all sorts of places, but not this change in particular, which is the problem.
But I think testing this as an API (by checking that all names resolve to the correct arch) is too trivial to be worth doing it.
So I recommend you submit the actual change you need this for, and only what you need from this patch on it. If the new change has tests (compiling, linking) on the arch you need, than that'll be the test.
It's unclear to me why you have to add `x86_64` after adding new cases to the switch, so I suggest you don't add more than what you need on your actual patch.
I think once we know where this is coming from and why you need it, and you have tests for that, we'll be comfortable in knowing this is being tested.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122003/new/
https://reviews.llvm.org/D122003
More information about the llvm-commits
mailing list