[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