[PATCH] D31359: [GlobalISel]: Allow backends to custom legalize Intrinsics

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added a comment.

In https://reviews.llvm.org/D31359#712612, @aditya_nandakumar wrote:

> In https://reviews.llvm.org/D31359#712574, @t.p.northover wrote:
>
> > > The other (easier) approach is to reportGlobalISelFailure on intermediate instructions whenever we're unable to legalize/Lower them.
> >
> > Is this all accounting for the fact that you may not know if you can legalize an intrinsic before you've already erased it? Seems like a bit of an odd situation to be in if so.
>
>
> Not sure if I understand the question correctly. My view is the Intrinsic was legalized by the backend (created some machine instruction, erased the generic intrinsic and created  intermediate instructions). It's entirely possible that the intermediate instructions may fail legalization (for e.g. missing LegalizeActions/custom lowering of other intrinsics etc).


FWIW, I think I've seen similar situations while working on my patch for non-power-of-2-patches at https://reviews.llvm.org/D30529.
For example, I think I've seen this while working on that patch when legalizing e.g. <3 x i3> in a number of steps (<3 x i3>-> <3 x i8>, then <3 x i8> -> <8 x i8>).
If the <3 x i3> -> <3 x i8> step succeeds and the second <3 x i8> -> <8 x i8> step fails, the original instruction was indeed removed already.
I think I saw the reportGISelFailure call to then try and access the already erased instruction, leading to a segfault.
I don't fully remember the details and may have misremembered some of this above. If it'd be useful I can dig deeper to see if I could reproduce the issue.

It'd be nice if there could be a test case for the fallback mechanism in-tree for a case like this. It's not immediately obvious to me how such an in-tree test could be created at the moment.
All in all, my impression so far is that patch has 2 changes:

1. Having a mechanism to not blow up reportGISelFailure when a multi-step legalization fails half-way through
2. Allowing custom legalization of intrinsics.

If I've understood correctly, maybe best to commit them separately once there's agreement?


Repository:
  rL LLVM

https://reviews.llvm.org/D31359