From fccde203987bae8c01cad6d4d825d87f668a5de6 Mon Sep 17 00:00:00 2001 From: Eldritch Conundrum Date: Thu, 18 Apr 2024 15:38:26 -0700 Subject: [PATCH] Peephole optims, var scoping (fixes #153) (#349) Co-authored-by: Laurent Le Brun --- src/analyzer.fs | 4 +- src/rewriter.fs | 63 ++++++++++++++----- tests/compression_results.log | 16 ++--- tests/real/buoy.frag.expected | 3 +- .../real/controllable-machinery.frag.expected | 18 ++---- tests/real/ed-209.frag.expected | 3 +- tests/real/frozen-wasteland.frag.expected | 3 +- tests/real/kinder_painter.expected | 3 +- tests/real/robin.frag.expected | 3 +- tests/real/terrarium.frag.expected | 2 +- tests/unit/bug321.expected | 6 +- tests/unit/bug321.frag | 4 +- tests/unit/conditionals.frag.expected | 14 ++--- tests/unit/inline-aggro.aggro.expected | 5 +- tests/unit/inline-aggro.expected | 3 +- tests/unit/inline.aggro.expected | 13 +++- tests/unit/inline.expected | 13 +++- tests/unit/inline.frag | 13 ++++ tests/unit/inline.no.expected | 14 ++++- tests/unit/operators.expected | 6 +- tests/unit/shadowing.frag | 49 +++++++++++++++ tests/unit/shadowing.frag.expected | 21 +++++++ 22 files changed, 199 insertions(+), 80 deletions(-) diff --git a/src/analyzer.fs b/src/analyzer.fs index 93458f63..03cb72d8 100644 --- a/src/analyzer.fs +++ b/src/analyzer.fs @@ -74,10 +74,10 @@ module private VariableInlining = if not ident.DoNotInline && not ident.ToBeInlined && not ident.VarDecl.Value.isEverWrittenAfterDecl then match localReferences.TryGetValue(def.Key), allReferences.TryGetValue(def.Key) with | (_, 1), (_, 1) when isConst -> - debug $"inlining local variable '{Printer.debugIdent ident}' because it's safe to inline and used only once" + debug $"inlining local variable '{Printer.debugIdent ident}' because it's safe to inline (const) and used only once" ident.ToBeInlined <- true | (_, 0), (_, 0) -> - debug $"inlining local variable '{Printer.debugIdent ident}' because it's safe to inline and unused" + debug $"inlining (removing) local variable '{Printer.debugIdent ident}' because it's safe to inline and unused" ident.ToBeInlined <- true | _ -> () diff --git a/src/rewriter.fs b/src/rewriter.fs index 6fec8fb9..1e93317d 100644 --- a/src/rewriter.fs +++ b/src/rewriter.fs @@ -11,6 +11,15 @@ let renameField field = field |> String.map (fun c -> options.canonicalFieldNames.[swizzleIndex c]) else field +let rec private isPure = function + | Var v when v.Name = "true" || v.Name = "false" -> true + | Int _ + | Float _ -> true + | FunCall(Var fct, args) -> + Builtin.pureBuiltinFunctions.Contains fct.Name && List.forall isPure args + | FunCall(Op op, args) -> not (Builtin.assignOps.Contains op) && List.forall isPure args + | _ -> false + module private RewriterImpl = // Remove useless spaces in macros @@ -410,35 +419,62 @@ module private RewriterImpl = | Decl (_, []) -> false | _ -> true) + let exprUsesIdentName expr identName = + let mutable idents = [] + let collectLocalUses _ = function + | Var v as e -> idents <- v :: idents; e + | e -> e + mapExpr (mapEnv collectLocalUses id) expr |> ignore + idents |> List.exists (fun i -> i.Name = identName) + // Merge two consecutive items into one, everywhere possible in a list. - let rec squeeze (f : 'a * 'a -> 'a option) = function + let rec squeeze (f : 'a * 'a -> 'a list option) = function | h1 :: h2 :: t -> match f (h1, h2) with - | Some x -> squeeze f (x :: t) + | Some xs -> squeeze f (xs @ t) | None -> h1 :: (squeeze f (h2 :: t)) | h :: t -> h :: t | [] -> [] - - // Merge preceding expression into a for's init. let b = b |> squeeze (function + // Merge preceding expression into a for's init. | (Expr e, ForE (None, cond, inc, body)) -> // a=0;for(;i<5;++i); -> for(a=0;i<5;++i); - Some (ForE (Some e, cond, inc, body)) + Some [ForE (Some e, cond, inc, body)] | (Expr e, While (cond, body)) -> // a=0;while(i<5); -> for(a=0;i<5;); - Some (ForE(Some e, Some cond, None, body)) + Some [ForE(Some e, Some cond, None, body)] | Decl (_, [declElt]), Jump(JumpKeyword.Return, Some (Var v)) // int x=f();return x; -> return f(); when v.Name = declElt.name.Name && declElt.init.IsSome -> - Some (Jump(JumpKeyword.Return, declElt.init)) + Some [Jump(JumpKeyword.Return, declElt.init)] | Expr (FunCall(Op "=", [Var v1; e])), Jump(JumpKeyword.Return, Some (Var v2)) // x=f();return x; -> return f(); when v1.Name = v2.Name -> match v1.VarDecl with | Some d -> if d.scope <> VarScope.Global && not (d.ty.isOutOrInout) then - Some (Jump(JumpKeyword.Return, Some e)) + Some [Jump(JumpKeyword.Return, Some e)] else None | _ -> None + // Remove unused assignment immediately followed by re-assignment: m=14.;m=58.; -> 14.;m=58.; + | Expr (FunCall (Op "=", [Var name; init1])), (Expr (FunCall (Op "=", [Var name2; init2])) as assign2) + when name.Name = name2.Name && not (exprUsesIdentName init2 name.Name) -> + match name.Declaration with + | Declaration.Variable decl when decl.scope = VarScope.Global -> + // The assignment should not be removed if init2 calls a function that reads the global variable. + None // Safely assume it could happen. + | Declaration.Variable _ -> Some [Expr init1; assign2] // Transform is safe even if the var is an out parameter. + | _ -> None + // Compact a pure declaration immediately followed by re-assignment: float m=14.;m=58.; -> float m=58.; + | Decl (ty, [declElt]), (Expr (FunCall (Op "=", [Var name2; init2])) as assign2) + when declElt.name.Name = name2.Name + && not (exprUsesIdentName init2 declElt.name.Name) + && declElt.init |> Option.map isPure |> Option.defaultValue true -> + Some [Decl (ty, [{declElt with init = Some init2}])] | _ -> None) + // Remove pure expression statements. + let b = b |> List.filter (function + | Expr e when isPure e -> false + | _ -> true) + // Inline inner decl-less blocks. (Presence of decl could lead to redefinitions.) a();{b();}c(); -> a();b();c(); let b = b |> List.collect (function | Block b when hasNoDecl b -> b @@ -478,7 +514,7 @@ module private RewriterImpl = let simplifyStmt = function | Block [] as e -> e | Block b -> match simplifyBlock b with - | [stmt] -> stmt + | [stmt] as b when hasNoDecl b -> stmt | stmts -> Block stmts | Decl (ty, li) -> Decl (rwType ty, declsNotToInline li) | ForD ((ty, d), cond, inc, body) -> ForD((rwType ty, declsNotToInline d), cond, inc, squeezeBlockWithComma body) @@ -584,14 +620,7 @@ let reorderFunctions code = // Inline the argument of a function call into the function body. module private ArgumentInlining = - let rec isInlinableExpr = function - | Var v when v.Name = "true" || v.Name = "false" -> true - | Int _ - | Float _ -> true - | FunCall(Var fct, args) -> - Builtin.pureBuiltinFunctions.Contains fct.Name && List.forall isInlinableExpr args - | FunCall(Op op, args) -> not (Builtin.assignOps.Contains op) && List.forall isInlinableExpr args - | _ -> false + let isInlinableExpr e = isPure e type [] Inlining = { func: TopLevel diff --git a/tests/compression_results.log b/tests/compression_results.log index e9624fa9..44a664ec 100644 --- a/tests/compression_results.log +++ b/tests/compression_results.log @@ -1,24 +1,24 @@ clod.frag... 8897 => 1528.145 mouton/mouton.vert... 17026 => 2457.862 audio-flight-v2.frag 4538 => 891.404 -buoy.frag 4097 => 624.156 -controllable-machinery.frag 7720 => 1220.140 -ed-209.frag 7768 => 1340.423 +buoy.frag 4094 => 622.550 +controllable-machinery.frag 7708 => 1220.329 +ed-209.frag 7766 => 1341.089 elevated.hlsl 3405 => 603.218 endeavour.frag 2605 => 534.116 from-the-seas-to-the-stars.frag 14292 => 2328.094 -frozen-wasteland.frag 4583 => 806.518 -kinder_painter.frag 2867 => 447.669 +frozen-wasteland.frag 4578 => 806.452 +kinder_painter.frag 2865 => 445.014 leizex.frag 2298 => 510.191 lunaquatic.frag 5247 => 1049.030 mandelbulb.frag 2370 => 538.322 ohanami.frag 3256 => 722.517 orchard.frag 5537 => 1022.773 oscars_chair.frag 4651 => 986.364 -robin.frag 6297 => 1053.367 +robin.frag 6295 => 1052.336 slisesix.frag 4573 => 931.855 -terrarium.frag 3626 => 746.585 +terrarium.frag 3624 => 745.827 the_real_party_is_in_your_pocket.frag 12111 => 1794.687 valley_ball.glsl 4386 => 888.496 yx_long_way_from_home.frag 2947 => 606.406 -Total: 135097 => 23632.338 +Total: 135069 => 23627.076 diff --git a/tests/real/buoy.frag.expected b/tests/real/buoy.frag.expected index 73228a54..a3bd7106 100644 --- a/tests/real/buoy.frag.expected +++ b/tests/real/buoy.frag.expected @@ -204,8 +204,7 @@ void mainImage(out vec4 fragColor,vec2 fragCoord) vec3 pos,ray; CamPolar(pos,ray,camRot,fragCoord); float to=TraceOcean(pos,ray),tb=TraceBoat(pos,ray); - vec3 result; - result=to>0.&&(to0.&&(to0.? ShadeBoat(pos+ray*tb,ray): diff --git a/tests/real/controllable-machinery.frag.expected b/tests/real/controllable-machinery.frag.expected index e2d09f68..f5353dc7 100644 --- a/tests/real/controllable-machinery.frag.expected +++ b/tests/real/controllable-machinery.frag.expected @@ -40,8 +40,7 @@ vec4 BPos(float t) } float PrBoxDf(vec3 p,vec3 b) { - vec3 d; - d=abs(p)-b; + vec3 d=abs(p)-b; return min(max(d.x,max(d.y,d.z)),0.)+length(max(d,0.)); } float PrRoundBoxDf(vec3 p,vec3 b) @@ -78,8 +77,7 @@ float Maxv2(vec2 p) } float SmoothMin(float a,float b,float r) { - float h; - h=clamp(.5+.5*(b-a)/r,0.,1.); + float h=clamp(.5+.5*(b-a)/r,0.,1.); return mix(b-h*r,a,h); } float SmoothMax(float a,float b,float r) @@ -100,8 +98,7 @@ mat3 StdVuMat(float el,float az) } vec2 Rot2D(vec2 q,float a) { - vec2 cs; - cs=sin(a+vec2(1.57079635,0)); + vec2 cs=sin(a+vec2(1.57079635,0)); return vec2(dot(q,vec2(cs.x,-cs.y)),dot(q.yx,cs)); } float GearWlDf(vec3 p,float rad,float wlThk,float tWid,float nt,float aRot,bool bev,float dMin) @@ -241,8 +238,7 @@ float GearRay(vec3 ro,vec3 rd) vec3 GearNf(vec3 p) { vec4 v; - vec2 e; - e=vec2(5e-4,-5e-4); + vec2 e=vec2(5e-4,-5e-4); for(int j=VAR_ZERO;j<4;j++) v[j]=GearDf(p+(j<2? j==0? @@ -257,8 +253,7 @@ vec3 GearNf(vec3 p) vec3 ObjNf(vec3 p) { vec4 v; - vec2 e; - e=vec2(5e-4,-5e-4); + vec2 e=vec2(5e-4,-5e-4); for(int j=VAR_ZERO;j<4;j++) v[j]=ObjDf(p+(j<2? j==0? @@ -447,8 +442,7 @@ vec3 ShowScene(vec3 ro,vec3 rd) } vec4 Loadv4(int idVar) { - float fi; - fi=float(idVar); + float fi=float(idVar); return texture(txBuf,(vec2(mod(fi,128.),floor(fi/128.))+.5)/txSize); } void mainImage(out vec4 fragColor,vec2 fragCoord) diff --git a/tests/real/ed-209.frag.expected b/tests/real/ed-209.frag.expected index 8cc30077..af14e450 100644 --- a/tests/real/ed-209.frag.expected +++ b/tests/real/ed-209.frag.expected @@ -274,8 +274,7 @@ MarchData room(vec3 p) r.d=min(backWall,max(length(p.z),-doorHole+.1)); if(r.d==backWall) { - float ocp; - ocp=min(abs(sdOctogon(xy,2.6)),abs(sdOctogon(xy,1.9))); + float ocp=min(abs(sdOctogon(xy,2.6)),abs(sdOctogon(xy,1.9))); ocp=max(ocp,min(.7-abs(xy.x+1.2),-xy.y)); ocp=min(ocp,max(abs(sdOctogon(xy,1.2)),min(xy.x,.7-abs(xy.y)))); if(ocp<.3) diff --git a/tests/real/frozen-wasteland.frag.expected b/tests/real/frozen-wasteland.frag.expected index 03544925..477e84f4 100644 --- a/tests/real/frozen-wasteland.frag.expected +++ b/tests/real/frozen-wasteland.frag.expected @@ -234,8 +234,7 @@ vec3 Sky(vec3 rd,vec3 ligt) } float Occ(vec3 p) { - float h=0.; - h=clamp(map(p),.5,1.); + float h=clamp(map(p),.5,1.); return sqrt(h); } void mainImage(out vec4 fragColor,vec2 fragCoord) diff --git a/tests/real/kinder_painter.expected b/tests/real/kinder_painter.expected index 325702c7..d4e8a23a 100644 --- a/tests/real/kinder_painter.expected +++ b/tests/real/kinder_painter.expected @@ -22,8 +22,7 @@ "float t(vec4 z,vec3 m,vec3 v)" "{" "vec3 r=m-z.xyz;" - "float w=dot(v.xz,v.xz),d=dot(v.xz,r.xz),b;" - "b=d*d-w*(dot(r.xz,r.xz)-z.w*z.w);" + "float w=dot(v.xz,v.xz),d=dot(v.xz,r.xz),b=d*d-w*(dot(r.xz,r.xz)-z.w*z.w);" "if(b>0.)" "b=(-d-sqrt(b))/w;" "return b-.001;" diff --git a/tests/real/robin.frag.expected b/tests/real/robin.frag.expected index 0e2c9160..43b3588e 100644 --- a/tests/real/robin.frag.expected +++ b/tests/real/robin.frag.expected @@ -35,8 +35,7 @@ float noise(vec3 x) } float fbm(vec3 p) { - float f; - f=1.6*noise(p); + float f=1.6*noise(p); p*=2.02; f+=.35*noise(p); p*=2.33; diff --git a/tests/real/terrarium.frag.expected b/tests/real/terrarium.frag.expected index 9e82f4df..8207b3ec 100644 --- a/tests/real/terrarium.frag.expected +++ b/tests/real/terrarium.frag.expected @@ -95,7 +95,7 @@ void main() else if(r<1.07) col=mix(col,0.,.2),p.xy=p.xy+vec2(-.5,-.866025),p=S(p); else - col=mix(col,0.,.2),col=1.,p.xy=p.xy+vec2(-.5,.866025),p=S(p),p.z+=.1; + mix(col,0.,.2),col=1.,p.xy=p.xy+vec2(-.5,.866025),p=S(p),p.z+=.1; if(time>48.) p.z=mix(p.z,mod(p.z+time/2.+2,4.)-2,smoothstep(48.,49.,time)); } diff --git a/tests/unit/bug321.expected b/tests/unit/bug321.expected index f122492e..683dd23e 100644 --- a/tests/unit/bug321.expected +++ b/tests/unit/bug321.expected @@ -1,13 +1,13 @@ -int n; +float n; float a(float s) { s=3.; n=2.; return s; } -float main() +float b() { float t=3.; t-=1.; return a(1.)+a(5.); -} \ No newline at end of file +} diff --git a/tests/unit/bug321.frag b/tests/unit/bug321.frag index 9cb5bfb1..e10b6fbf 100644 --- a/tests/unit/bug321.frag +++ b/tests/unit/bug321.frag @@ -1,4 +1,4 @@ -int n; +float n; const float s = 1.; float a(float s) { @@ -6,7 +6,7 @@ float a(float s) n = 2.; return s; } -float main() +float b() { float t = 3.; t -= s; diff --git a/tests/unit/conditionals.frag.expected b/tests/unit/conditionals.frag.expected index ed0b725e..6cc7921b 100644 --- a/tests/unit/conditionals.frag.expected +++ b/tests/unit/conditionals.frag.expected @@ -43,16 +43,14 @@ bool and3() } float ifStmtToExpr(float f) { - float r,r2,r3; - r=f>0.? - 1.: - 2.; - r2=f>1.? - 1.: - (1,2.); + float r3; if(f>1.) r3=1.; else r3=2.,1; - return r+r2+r3; + return(f>0.? + 1.: + 2.)+(f>1.? + 1.: + (1,2.))+r3; } diff --git a/tests/unit/inline-aggro.aggro.expected b/tests/unit/inline-aggro.aggro.expected index 620c675c..edd754e7 100644 --- a/tests/unit/inline-aggro.aggro.expected +++ b/tests/unit/inline-aggro.aggro.expected @@ -28,10 +28,7 @@ float inl6() return 2.*baz; } void notevil() -{ - float x=101.; - x=42.; -} +{} float inl7() { notevil(); diff --git a/tests/unit/inline-aggro.expected b/tests/unit/inline-aggro.expected index cf34816d..ba4dcdda 100644 --- a/tests/unit/inline-aggro.expected +++ b/tests/unit/inline-aggro.expected @@ -29,8 +29,7 @@ float inl6() } void notevil() { - float x=101.; - x=42.; + float x=42.; } float inl7() { diff --git a/tests/unit/inline.aggro.expected b/tests/unit/inline.aggro.expected index ee748cb7..ecd6be3e 100644 --- a/tests/unit/inline.aggro.expected +++ b/tests/unit/inline.aggro.expected @@ -35,8 +35,6 @@ float multiPass2() } int dont_inline_lvalue() { - int a=1; - a=2; return 3; } vec4 fragColor247; @@ -79,3 +77,14 @@ float inline_uninitialized() { return c; } +float glo; +float noinline_readsTheGlobal() +{ + return glo; +} +float dontCompressAssigments() +{ + glo=10.; + glo=50.+noinline_readsTheGlobal(); + return glo*glo; +} diff --git a/tests/unit/inline.expected b/tests/unit/inline.expected index 7a622911..f5b79003 100644 --- a/tests/unit/inline.expected +++ b/tests/unit/inline.expected @@ -35,8 +35,6 @@ float multiPass2() } int dont_inline_lvalue() { - int a=1; - a=2; return 3; } vec4 fragColor247; @@ -80,3 +78,14 @@ float inline_uninitialized() { return c; } +float glo; +float noinline_readsTheGlobal() +{ + return glo; +} +float dontCompressAssigments() +{ + glo=10.; + glo=50.+noinline_readsTheGlobal(); + return glo*glo; +} diff --git a/tests/unit/inline.frag b/tests/unit/inline.frag index 2001a6dd..386a0819 100644 --- a/tests/unit/inline.frag +++ b/tests/unit/inline.frag @@ -122,3 +122,16 @@ float inline_uninitialized() float c; return c; } + +// repro for a bug +float glo; +float noinline_readsTheGlobal() +{ + return glo; +} +float dontCompressAssigments() +{ + glo = 10.; + glo = 50. + noinline_readsTheGlobal(); + return glo*glo; +} diff --git a/tests/unit/inline.no.expected b/tests/unit/inline.no.expected index f30ebdcc..954f1086 100644 --- a/tests/unit/inline.no.expected +++ b/tests/unit/inline.no.expected @@ -40,8 +40,7 @@ float multiPass2() } int dont_inline_lvalue() { - int a=1; - a=2; + int a=2; return 3; } vec4 fragColor247; @@ -90,3 +89,14 @@ float inline_uninitialized() float c; return c; } +float glo; +float noinline_readsTheGlobal() +{ + return glo; +} +float dontCompressAssigments() +{ + glo=10.; + glo=50.+noinline_readsTheGlobal(); + return glo*glo; +} diff --git a/tests/unit/operators.expected b/tests/unit/operators.expected index cf63c909..f5a8a5c6 100644 --- a/tests/unit/operators.expected +++ b/tests/unit/operators.expected @@ -11,11 +11,7 @@ "void main()" "{" "int a=11,b=7,c=2;" - "float f=2.621464704,g,z;" - "f=.7;" - "f=1.4/3.;" - "g=mod(8.,3.);" - "z=122121;" + "float f=1.4/3.,g=mod(8.,3.),z=122121;" "}" "int no_parens(int a,int b,int c)" "{" diff --git a/tests/unit/shadowing.frag b/tests/unit/shadowing.frag index 7cafb283..946addd3 100644 --- a/tests/unit/shadowing.frag +++ b/tests/unit/shadowing.frag @@ -12,4 +12,53 @@ float g(float f) f+=g; } return f; +} + +#define noinline_random 4 + +int m1() +{ + int x = 1; + if (noinline_random > 3) + { + int x = 2, y = x; // y is initialized to 2 + return y; + } +} + +struct S{ int x; }; + +S m2() +{ + S S = S(0); // 'S' is only visible as a struct and constructor + return S; // 'S' is now visible as a variable +} + +int m4(int k) +{ + //int k = k + 3; // redeclaration error of the name k + { + int k = k + 3; // 2nd k is parameter, initializing nested first k + int m = k; // use of new k, which is hiding the parameter + } + return k; + + //int x = x; // Error if x has not been previously defined. + // If the previous definition of x was in this + // same scope, this causes a redeclaration error. +} + +float m5(float k) +{ + float m = 9.; + m = 14.; + { + float k = k + 3.; // 2nd k is parameter, initializing nested first k + m = k; // use of new k, which is hiding the parameter + } + return k + 2. * m; +} +void m6() +{ + m5(55.); } \ No newline at end of file diff --git a/tests/unit/shadowing.frag.expected b/tests/unit/shadowing.frag.expected index 7e840d30..88c10734 100644 --- a/tests/unit/shadowing.frag.expected +++ b/tests/unit/shadowing.frag.expected @@ -13,3 +13,24 @@ float g(float f) } return f; } + +#define noinline_random 4 + +int m1() +{ + if(noinline_random>3) + return 2; +} +struct S{int x;}; +S m2() +{ + return S(0); +} +int m4(int k) +{ + return k; +} +void m6() +{ + 171.; +}