On Mon, 14 Oct 2024 15:04:54 GMT, Jasmine Karthikeyan 
<jkarthike...@openjdk.org> wrote:

> For the record I think in this PR we could simply match the IR patterns in 
> the ad file, since (from my understanding) the patterns we are matching could 
> be supported there. We should do platform-specific lowering in a separate 
> patch because it is pretty nuanced, and we could potentially move it to the 
> new system afterwards.

Hi @jaskarth , Bigger pattern matching is sensitive to [IR level node 
sharing](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/matcher.cpp#L1724),
 thus it may not full proof for above 4 patterns. Current patch takes care of 
this limitation. 

> @jatin-bhateja That is machine-independent lowering, we are talking about 
> machine-dependent lowering to which `MacroLogicV` transformation belongs. You 
> can have `phaselowering_x86` and not have to add another method to `Matcher` 
> as well as add default implementations to various architecture files. You can 
> reuse `MulVL` node for that but I believe these transformations should be 
> done as late as possible.

Hi @merykitty, I see some scope of refactoring and carving out a separate 
target specific lowering pass going forward, I have brough this up in past too. 
Existing optimizations are in line with current infrastructure and guards 
target specific optimizations with target specific match_rule_supported checks 
e.g. 
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/compile.cpp#L2898.
 As @jaskarth suggests we can pick this up going forward.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21244#issuecomment-2411884206

Reply via email to