[code-reflection] RFR: [hat][codegen] Parenthesis in Expressions for binaryOps fixed

[Gary Frost]

On Mon, 22 Sep 2025 13:22:31 GMT, Juan Fumero <jfumero at openjdk.org> wrote:

> HAT Code gen was not generated parenthesis correctly. This PR forces to use an open and closed parenthesis for binaryOps. 
> 
> Code reflection generates code models in SSA form. Therefore, priority is given by computing values first the values before carrying out the result-op to other ops. 
> 
> For instance: 
> 
> 
> final int TN = 2;
> final int TF = 128;
> final int MAX = 1024;
> int c = MAX / (TN * TF);
> 
> 
> Generates the following code model:
> 
> 
>     %10 : java.type:"int" = var.load %9 @loc="50:17";
>     %11 : java.type:"int" = var.load %5 @loc="50:24";
>     %12 : java.type:"int" = var.load %7 @loc="50:29";
>     %13 : java.type:"int" = mul %11 %12 @loc="50:24";    >> mult
>     %14 : java.type:"int" = div %10 %13 @loc="50:17";      >> div
>     %15 : Var<java.type:"int"> = var %14 @loc="50:9" @"c";
> 
> 
> while 
> 
> 
> final int TN = 2;
> final int TF = 128;
> final int MAX = 1024;
> int c = MAX / TN * TF;
> 
> 
> Generates the following code model.
> 
> 
>     %10 : java.type:"int" = var.load %9 @loc="50:17";
>     %11 : java.type:"int" = var.load %5 @loc="50:23";
>     %12 : java.type:"int" = div %10 %11 @loc="50:17";
>     %13 : java.type:"int" = var.load %7 @loc="50:28";
>     %14 : java.type:"int" = mul %12 %13 @loc="50:17";
>     %15 : Var<java.type:"int"> = var %14 @loc="50:9" @"c";
> 
> 
> The issue was that in the HAT codegen, parentheses were computed based on pure precedence of the operator, not based on the dependencies. 
> 
> How to test:
> 
> 
> HAT=SHOW_CODE java @hat/test ffi-opencl oracle.code.hat.TestParenthesis

You are always adding parenthesis here, whether needed or not.  It sounds like you found a bug un the paranthesisIfNeeded (which should be checking C/Java style precedence against the parent).  Creating '(' and ')' seems like a noisy solution... (and a bit lazy... if I may be so bold ;) ) 

I think it is worth checking why the precedence check for these ops fail? Where thay have worked for 100's of binary ops in the past?

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

PR Comment: https://git.openjdk.org/babylon/pull/579#issuecomment-3319072274