Integrated: 8335880: More troubleshooting tips around windows space in path

2024-10-26 Thread Chen Liang
On Tue, 16 Jul 2024 15:38:13 GMT, Chen Liang  wrote:

> Context: https://mail.openjdk.org/pipermail/build-dev/2024-July/045586.html
> 
> People were confused on a few details around fixing windows space names, 
> including:
> 1. setshortname can report confusing error message when the directory is in 
> use
> 2. Many directories can have names set but only Microsoft Visual Studio and 
> Windows Kits need them
> 
> Also adds some notes about `jdk` being the exploded image while `images/jdk` 
> is the actual image.

This pull request has now been integrated.

Changeset: 762a573e
Author:Chen Liang 
URL:   
https://git.openjdk.org/jdk/commit/762a573ef1f4d800b98d3acfcc72c0b2792de69e
Stats: 23 lines in 2 files changed: 16 ins; 0 del; 7 mod

8335880: More troubleshooting tips around windows space in path

Reviewed-by: erikj, ihse

-

PR: https://git.openjdk.org/jdk/pull/20197


Re: RFR: 8342662: C2: Add new phase for backend-specific lowering [v2]

2024-10-26 Thread Jatin Bhateja
On Sat, 26 Oct 2024 02:34:44 GMT, Quan Anh Mai  wrote:

>>> By allowing lowering to look through VectorReinterpret and break the 
>>> invariant of Extract nodes that the element types of their inputs and 
>>> outputs must be the same, we can gvn v1 and v, v2 and v0.
>> 
>> I'd warn against breaking existing IR invariants. As an example, precise 
>> type information is important to properly match generic ideal vector nodes.
>
> I believe the matcher only needs the exact type of the node but not its 
> inputs. E.g. it should not be an issue if we `AddVB` a `vector` and a 
> `vector` into a `vector`.

Generic vector operand resolution cocretizes generic operands based on type 
agnostic node size, its a post matcher pass,  and its job is to replace generic 
MachOper operand nodes with cocrete ones (vec[SDXYZ]) which holds precise 
register mask needed by register allocator.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1817960219


Re: RFR: 8342662: C2: Add new phase for backend-specific lowering [v2]

2024-10-26 Thread Jatin Bhateja
On Thu, 24 Oct 2024 02:14:46 GMT, Jasmine Karthikeyan 
 wrote:

>> Build changes look good (but would be slightly better without the extra 
>> blank line). I have not reviewed the actual hotspot changes.
>
> Thanks for looking at the build changes, @magicus! I've pushed a commit that 
> removes the extra newline in the makefiles and adds newlines to the ends of 
> files that were missing them.
> 
> Thanks for taking a look as well, @merykitty and @jatin-bhateja! I've pushed 
> a commit that should address the code suggestions and left some comments as 
> well.

> @jaskarth thanks for exploring platform-specific lowering!
> 
> I briefly looked through the changes, but I didn't get a good understanding 
> of its design goals. It's hard to see what use cases it is targeted for when 
> only skeleton code is present. It would really help if there are some cases 
> ported on top of it for illustration purposes and to solidify the design.
> 
> Currently, there are multiple places in the code where IR lowering happens. 
> In particular:
> 
> * IGVN during post loop opts phase (guarded by 
> `Compile::post_loop_opts_phase()`) (Ideal -> Ideal);
> * macro expansion (Ideal -> Ideal);
> * ad-hoc passes (GC barrier expansion, `MacroLogicV`) (Ideal -> Ideal);
> * final graph reshaping (Ideal -> Ideal);
> * matcher (Ideal -> Mach).
> 
> I'd like to understand how the new pass is intended to interact with existing 
> cases.
> 
> Only the last one is truly platform-specific, but there are some 
> platform-specific cases exposes in other places (e.g., MacroLogicV pass, 
> DivMod combining) guarded by some predicates on `Matcher`.
> 
> As the `PhaseLowering` is implemented now, it looks like a platform-specific 
> macro expansion pass (1->many rewriting). But `MacroLogicV` case doesn't fit 
> such model well.
> 




> I see changes to enable platform-specific node classes. As of now, only 
> Matcher introduces platform-specific nodes and all of them are Mach nodes. 
> Platform-specific Ideal nodes are declared in shared code, but then their 
> usages are guarded by `Matcher::has_match_rule()` thus ensuring there's 
> enough support on back-end side.
> 
> Some random observations:
> 
> * the pass is performed unconditionally and it iterates over all live nodes; 
> in contrast, macro nodes and nodes for post-loop opts IGVN are explicitly 
> listed on the side (MacroLogicV pass is also guarded, but by a coarse-grained 
> check);

MacroLogicV currently is a standalone pass and was not implemented as discrete 
Idealizations split across various logic IR Idealizations, our intention was to 
create a standalone pass and limit the exposure of such complex logic packing 
to rest of the code and  guard it by target specific match_rule_supported 
check. 
To fit this into new lowering interface, we may need to split root detection 
across various logic IR node accepted by PhaseLowering::lower_to(Node*).

@jaskarth, @merykitty , can you share more use cases apart from VPMUL[U]DQ and 
MacroLogicV detection in support of new lowring pass. Please be aware that even 
after introducing lowering pass we still want to prevent replcating IR 
transforms, e.g transformations/Idealizations only applicable  to x86 and 
AARCH64 should still be shared for maintainability reasons. In general 
compilers having lowering pass also introduce target specific pre matching IR.

-

PR Comment: https://git.openjdk.org/jdk/pull/21599#issuecomment-2439800311