[PATCH] D74228: [PatternMatch] Match XOR variant of unsigned-add overflow check.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D74228#1877769 <https://reviews.llvm.org/D74228#1877769>, @lebedev.ri wrote:

> In D74228#1870877 <https://reviews.llvm.org/D74228#1870877>, @spatel wrote:
>
> > In D74228#1870646 <https://reviews.llvm.org/D74228#1870646>, @fhahn wrote:
> >
> > > So should I prepare a set of patches for the improved target hook? Or SelDag? I would slightly prefer CGP for that, also with GlobalISel in mind and that's were we already do the expansion for similar cases.
> >
> >
> > It's an odd argument to add to CGP if we are using GlobalISel for codegen. CGP is intended to be a SDAG-only hack because of the basic block limitation there. Without the block limitation, we should favor doing transforms in the "official" codegen form. So IIUC, GISel should never use CGP. Eventually, the GISel path must re-implement the combines in both CGP and SDAG to become perf-equivalent.
>
>
>
>
> > So I'd favor SDAG on this patch unless there's evidence that the 'not' and 'cmp' are not always in the same block?
>
> It is quite trivial to come up with a contrived example that shows this can reasonably happen:
>  https://godbolt.org/z/Jj92M9
>
> > @nikic or others - does that match your expectations?
>
> I admittedly haven't paid match attention to what's happening here,
>  but i agree that this pattern is missed, and do think we should handle multi-BB case.


Ok, then I think we can proceed with this patch in CGP, but as mentioned, I think we need to tighten the default TLI hook, so we don't create regressions for some targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74228