[PATCH] D121085: [AArch64][GlobalISel] Implement G_SELECT translate to min/max/abs
Amara Emerson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 12 21:10:26 PST 2022
aemerson added a comment.
In D121085#3371597 <https://reviews.llvm.org/D121085#3371597>, @yuntaopan wrote:
> In D121085#3366519 <https://reviews.llvm.org/D121085#3366519>, @foad wrote:
>
>> In D121085#3365851 <https://reviews.llvm.org/D121085#3365851>, @aemerson wrote:
>>
>>> Thanks for the patch. Unfortunately I see the code that this was ported from in SelectionDAGBuilder does the optimization there because it relies on ValueTracking's `matchSelectPattern()` infrastructure. @arsenm @foad do you think it's worth it here to replicate that functionality for generic MIR so we can avoid adding this to the IRTranslator?
>>
>> Shouldn't the select -> min/max/abs be done in IR, so there would be no need for this patch? See D98152 <https://reviews.llvm.org/D98152> for example.
>
> Our development branch does not have that patch yet, I have tried that patch and it works well. it's more appropriate to handle it at the IR and no need for this patch. But I have a question,why gisel can not rely on ValueTracking infrastructure ? Because gisel is based on mir, but ValueTracking is an IR analysis ?@aemerson
There's nothing wrong with using the ValueTracking infrastructure, the problem is that the only place we can use IR analysis are during the IRTranslator phase, and we try to avoid doing optimizations there as much as possible. Sometimes that might not be practical, but where we can do like to perform optimizations on generic MIR in combine passes instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121085/new/
https://reviews.llvm.org/D121085
More information about the llvm-commits
mailing list