Skip to content

Conversation

@gbaraldi
Copy link
Contributor

@gbaraldi gbaraldi commented Sep 5, 2025

We can change the names here but I've tested that this works (maybe we could register a callback and see if it gets invoked?)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2025

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.
diff --git a/deps/build_local.jl b/deps/build_local.jl
index 1f15f8be..09d378f4 100644
--- a/deps/build_local.jl
+++ b/deps/build_local.jl
@@ -47,7 +47,7 @@ LLVM_DIR = joinpath(LLVM.artifact_dir, "lib", "cmake", "llvm")
 
 # build and install
 @info "Building" source_dir scratch_dir build_dir LLVM_DIR
-cmake(adjust_LIBPATH=!Sys.iswindows()) do cmake_path
+cmake(adjust_LIBPATH = !Sys.iswindows()) do cmake_path
     config_opts = `-DLLVM_ROOT=$(LLVM_DIR) -DCMAKE_INSTALL_PREFIX=$(scratch_dir)`
     if Sys.iswindows()
         # prevent picking up MSVC
diff --git a/src/newpm.jl b/src/newpm.jl
index 56279d51..0609693f 100644
--- a/src/newpm.jl
+++ b/src/newpm.jl
@@ -516,26 +516,26 @@ function InternalizePass(; preserved_gvs::Vector=String[], kwargs...)
 end
 # Module callbacks (not part of general pass sweep)
 export PipelineStartCallbacks, PipelineEarlySimplificationCallbacks,
-       OptimizerEarlyCallbacks, OptimizerLastCallbacks
-function PipelineStartCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+    OptimizerEarlyCallbacks, OptimizerLastCallbacks
+function PipelineStartCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "pipeline-start-callbacks" * kwargs_to_params(opts)
+    return "pipeline-start-callbacks" * kwargs_to_params(opts)
 end
-function PipelineEarlySimplificationCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function PipelineEarlySimplificationCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "pipeline-early-simplification-callbacks" * kwargs_to_params(opts)
+    return "pipeline-early-simplification-callbacks" * kwargs_to_params(opts)
 end
-function OptimizerEarlyCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function OptimizerEarlyCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "optimizer-early-callbacks" * kwargs_to_params(opts)
+    return "optimizer-early-callbacks" * kwargs_to_params(opts)
 end
-function OptimizerLastCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function OptimizerLastCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "optimizer-last-callbacks" * kwargs_to_params(opts)
+    return "optimizer-last-callbacks" * kwargs_to_params(opts)
 end
 
 # CGSCC passes
@@ -551,10 +551,10 @@ end
 
 #CGSCC callbacks (not part of general pass sweep)
 export CGSCCOptimizerLateCallbacks
-function CGSCCOptimizerLateCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function CGSCCOptimizerLateCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "cgscc-optimizer-late-callbacks" * kwargs_to_params(opts)
+    return "cgscc-optimizer-late-callbacks" * kwargs_to_params(opts)
 end
 
 # function passes
@@ -742,25 +742,25 @@ end
 
 # Function pass callbacks (not part of general pass sweep)
 export PeepholeCallbacks, ScalarOptimizerLateCallbacks, VectorizerStartCallbacks
-function PeepholeCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function PeepholeCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "peephole-callbacks" * kwargs_to_params(opts)
+    return "peephole-callbacks" * kwargs_to_params(opts)
 end
-function ScalarOptimizerLateCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function ScalarOptimizerLateCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "scalar-optimizer-late-callbacks" * kwargs_to_params(opts)
+    return "scalar-optimizer-late-callbacks" * kwargs_to_params(opts)
 end
-function VectorizerStartCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function VectorizerStartCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "vectorizer-start-callbacks" * kwargs_to_params(opts)
+    return "vectorizer-start-callbacks" * kwargs_to_params(opts)
 end
 @static if version() >= v"21"
     export VectorizerEndCallbacks
-    function VectorizerEndCallbacks(; opt_level=0)
-        opts = Dict{Symbol,Any}()
+    function VectorizerEndCallbacks(; opt_level = 0)
+        opts = Dict{Symbol, Any}()
         opts[Symbol("O$opt_level")] = true
         "vectorizer-end-callbacks" * kwargs_to_params(opts)
     end
@@ -804,15 +804,15 @@ end
 
 # Loop Callbacks (not part of general pass sweep)
 export LateLoopOptimizationsCallbacks, LoopOptimizerEndCallbacks
-function LateLoopOptimizationsCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function LateLoopOptimizationsCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "late-loop-optimizations-callbacks" * kwargs_to_params(opts)
+    return "late-loop-optimizations-callbacks" * kwargs_to_params(opts)
 end
-function LoopOptimizerEndCallbacks(; opt_level=0)
-    opts = Dict{Symbol,Any}()
+function LoopOptimizerEndCallbacks(; opt_level = 0)
+    opts = Dict{Symbol, Any}()
     opts[Symbol("O$opt_level")] = true
-    "loop-optimizer-end-callbacks" * kwargs_to_params(opts)
+    return "loop-optimizer-end-callbacks" * kwargs_to_params(opts)
 end
 
 
diff --git a/test/newpm.jl b/test/newpm.jl
index ae5d6469..c76f0adf 100644
--- a/test/newpm.jl
+++ b/test/newpm.jl
@@ -195,12 +195,12 @@ end
     end
 
     @testset "loop" begin
-        # skip opt-level callback pseudo-passes, they require parameters and are provided as functions
-        skip_loop = [
-            "late-loop-optimizations-callbacks",
-            "loop-optimizer-end-callbacks",
-        ]
-        test_passes("loop", LLVM.loop_passes, skip_loop)
+            # skip opt-level callback pseudo-passes, they require parameters and are provided as functions
+            skip_loop = [
+                "late-loop-optimizations-callbacks",
+                "loop-optimizer-end-callbacks",
+            ]
+            test_passes("loop", LLVM.loop_passes, skip_loop)
     end
 end
 
@@ -414,31 +414,31 @@ end
     end
 end
 
-@testset "callbacks" begin
-    # just check that the callbacks can be registered and run without errors
-    @dispose ctx=Context() begin
-        # module callbacks
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!("pipeline-start-callbacks<O0>", mod) === nothing
-        end
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!(PipelineStartCallbacks(opt_level=0), mod) === nothing
-        end
-        # CGSCC callback
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!("cgscc-optimizer-late-callbacks<O0>", mod) === nothing
-        end
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!(CGSCCOptimizerLateCallbacks(opt_level=0), mod) === nothing
-        end
-        # function callbacks
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!("peephole-callbacks<O0>", mod) === nothing
-        end
-        @dispose pb=NewPMPassBuilder() mod=test_module() begin
-            @test run!(PeepholeCallbacks(opt_level=0), mod) === nothing
+    @testset "callbacks" begin
+        # just check that the callbacks can be registered and run without errors
+        @dispose ctx = Context() begin
+            # module callbacks
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!("pipeline-start-callbacks<O0>", mod) === nothing
+            end
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!(PipelineStartCallbacks(opt_level = 0), mod) === nothing
+            end
+            # CGSCC callback
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!("cgscc-optimizer-late-callbacks<O0>", mod) === nothing
+            end
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!(CGSCCOptimizerLateCallbacks(opt_level = 0), mod) === nothing
+            end
+            # function callbacks
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!("peephole-callbacks<O0>", mod) === nothing
+            end
+            @dispose pb = NewPMPassBuilder() mod = test_module() begin
+                @test run!(PeepholeCallbacks(opt_level = 0), mod) === nothing
+            end
         end
     end
-end
 
 end

@vchuravy
Copy link
Collaborator

vchuravy commented Sep 5, 2025

Would it make sense to open a PR upstream and ask for feedback?

@gbaraldi
Copy link
Contributor Author

gbaraldi commented Sep 5, 2025

I think so.

But we also need this here anyway.

@vchuravy
Copy link
Collaborator

vchuravy commented Sep 5, 2025

Sure, but before we commit to the API. Maybe we get lucky with a quick review ;)

@vchuravy
Copy link
Collaborator

Bump since this is now on LLVM main?

end

@testset "callbacks" begin
# just check that the callbacks can be registered and run without errors
Copy link
Collaborator

@maleadt maleadt Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about testing that the callbacks actually fire?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the callbacks run?

        @dispose pb=NewPMPassBuilder(debug_logging=true) mod=test_module() begin
            add!(pb, "pipeline-start-callbacks<O3>")
            add!(pb, "pipeline-early-simplification-callbacks<O3>")
            run!(pb, mod)
        end

This results in empty output...

# just check that the callbacks can be registered and run without errors
@dispose ctx=Context() begin
# module callbacks
@dispose pb=NewPMPassBuilder() mod=test_module() begin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pass builders are unused here.

@maleadt
Copy link
Collaborator

maleadt commented Oct 14, 2025

Should any of this be gated against LLVM < 22?

@gbaraldi
Copy link
Contributor Author

Yeah, I think that's when the upstream version will land

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants