Revert once flag to be a bool so we keep a single implementation

This avoids having two implementations and it will simplify the call
site when we protect class getter/property macros with lazy
initializers: the flag is always a Bool not either a
Crystal::Once::State or a Bool depending on the compiler.

Author: ysbaddaden

src/compiler/crystal/codegen/class_var.cr

@@ -25,8 +25,8 @@ class Crystal::CodeGenVisitor
     initialized_flag_name = class_var_global_initialized_name(class_var)
     initialized_flag = @main_mod.globals[initialized_flag_name]?
     unless initialized_flag
-      initialized_flag = @main_mod.globals.add(@main_llvm_context.int8, initialized_flag_name)
-      initialized_flag.initializer = @main_llvm_context.int8.const_int(0)
+      initialized_flag = @main_mod.globals.add(@main_llvm_context.int1, initialized_flag_name)
+      initialized_flag.initializer = @main_llvm_context.int1.const_int(0)
       initialized_flag.linkage = LLVM::Linkage::Internal if @single_module
       initialized_flag.thread_local = true if class_var.thread_local?
     end
@@ -61,7 +61,7 @@ class Crystal::CodeGenVisitor
       initialized_flag_name = class_var_global_initialized_name(class_var)
       initialized_flag = @llvm_mod.globals[initialized_flag_name]?
       unless initialized_flag
-        initialized_flag = @llvm_mod.globals.add(llvm_context.int8, initialized_flag_name)
+        initialized_flag = @llvm_mod.globals.add(llvm_context.int1, initialized_flag_name)
         initialized_flag.thread_local = true if class_var.thread_local?
       end
     end

src/compiler/crystal/codegen/const.cr

@@ -64,8 +64,8 @@ class Crystal::CodeGenVisitor
     initialized_flag_name = const.initialized_llvm_name
     initialized_flag = @main_mod.globals[initialized_flag_name]?
     unless initialized_flag
-      initialized_flag = @main_mod.globals.add(@main_llvm_context.int8, initialized_flag_name)
-      initialized_flag.initializer = @main_llvm_context.int8.const_int(0)
+      initialized_flag = @main_mod.globals.add(@main_llvm_context.int1, initialized_flag_name)
+      initialized_flag.initializer = @main_llvm_context.int1.const_int(0)
       initialized_flag.linkage = LLVM::Linkage::Internal if @single_module
     end
     initialized_flag

src/compiler/crystal/codegen/once.cr

@@ -40,9 +40,6 @@ class Crystal::CodeGenVisitor
       end
 
       state = load(once_state_type, once_state_global)
-      {% if LibLLVM::IS_LT_150 %}
-        flag = bit_cast(flag, @llvm_context.int1.pointer) # cast Int8* to Bool*
-      {% end %}
       args = [state, flag, initializer]
     end
 

src/crystal/once.cr

@@ -14,25 +14,13 @@ require "crystal/spin_lock"
 module Crystal
   # :nodoc:
   module Once
-    enum State : Int8
-      Processing    = -1
-      Uninitialized =  0
-      Initialized   =  1
-    end
-
-    {% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %}
-      alias FlagT = State
-    {% else %}
-      alias FlagT = Bool
-    {% end %}
-
     struct Operation
       include PointerLinkedList::Node
 
       getter fiber : Fiber
-      getter flag : FlagT*
+      getter flag : Bool*
 
-      def initialize(@flag : FlagT*, @fiber : Fiber)
+      def initialize(@flag : Bool*, @fiber : Fiber)
         @waiting = PointerLinkedList(Fiber::PointerLinkedListNode).new
       end
 
@@ -53,126 +41,105 @@ module Crystal
       @@operations = PointerLinkedList(Operation).new
     end
 
-    protected def self.exec(flag : FlagT*, &)
+    protected def self.exec(flag : Bool*, &)
       @@spin.lock
 
-      exec_impl(flag) { yield }
+      if flag.value
+        @@spin.unlock
+      elsif operation = processing?(flag)
+        check_reentrancy(operation)
+        wait_initializer(operation)
+      else
+        run_initializer(flag) { yield }
+      end
 
       # safety check, and allows to safely call `Intrinsics.unreachable` in
       # `__crystal_once`
-      if flag.is_a?(State*)
-        return if flag.value.initialized?
-      else
-        return if flag.value
-      end
+      return if flag.value
 
-      System.print_error "BUG: failed to initialize constant or class variable\n"
+      System.print_error "BUG: failed to initialize class variable or constant\n"
       LibC._exit(1)
     end
 
-    private def self.run_initializer(flag, &)
-      if flag.is_a?(State*)
-        flag.value = State::Processing
+    private def self.processing?(flag)
+      @@operations.each do |operation|
+        return operation if operation.value.flag == flag
       end
+    end
+
+    private def self.check_reentrancy(operation)
+      if operation.value.fiber == Fiber.current
+        @@spin.unlock
+        raise "Recursion while initializing class variables and/or constants"
+      end
+    end
+
+    private def self.wait_initializer(operation)
+      waiting = Fiber::PointerLinkedListNode.new(Fiber.current)
+      operation.value.add_waiter(pointerof(waiting))
+      @@spin.unlock
+      Fiber.suspend
+    end
+
+    private def self.run_initializer(flag, &)
       operation = Operation.new(flag, Fiber.current)
       @@operations.push pointerof(operation)
       @@spin.unlock
 
       yield
 
       @@spin.lock
-      if flag.is_a?(State*)
-        flag.value = State::Initialized
-      else
-        flag.value = true
-      end
+      flag.value = true
       @@operations.delete pointerof(operation)
       @@spin.unlock
 
       operation.resume_all
     end
-
-    # Searches if a fiber is already running the initializer, in which case it
-    # checks for reentrancy then suspends the fiber until the value is ready and
-    # returns true; otherwise immediately returns false.
-    private def self.wait_initializer?(flag) : Bool
-      @@operations.each do |operation|
-        next unless operation.value.flag == flag
-
-        current_fiber = Fiber.current
-
-        if operation.value.fiber == current_fiber
-          @@spin.unlock
-          raise "Recursion while initializing class variables and/or constants"
-        end
-
-        waiting = Fiber::PointerLinkedListNode.new(current_fiber)
-        operation.value.add_waiter(pointerof(waiting))
-        @@spin.unlock
-
-        Fiber.suspend
-        return true
-      end
-
-      false
-    end
   end
 
   # :nodoc:
+  #
+  # Never inlined to avoid bloating the call site with the slow-path that should
+  # usually not be taken.
   @[NoInline]
-  def self.once(flag : Once::FlagT*, initializer : Void*)
+  def self.once(flag : Bool*, initializer : Void*)
     Once.exec(flag, &Proc(Nil).new(initializer, Pointer(Void).null))
   end
-end
-
-{% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %}
-  module Crystal
-    module Once
-      private def self.exec_impl(flag, &)
-        case flag.value
-        in .initialized?
-          @@spin.unlock
-          return
-        in .uninitialized?
-          run_initializer(flag) { yield }
-        in .processing?
-          raise "unreachable" unless wait_initializer?(flag)
-        end
-      end
-    end
 
-    # :nodoc:
-    def self.once(flag : Once::State*, &)
-      Once.exec(flag) { yield } unless flag.value.initialized?
-    end
+  # :nodoc:
+  #
+  # NOTE: should also never be inlined, but that would capture the block, which
+  # would be a breaking change when we use this method to protect class getter
+  # and class property macros with lazy initialization (the block may return or
+  # break).
+  #
+  # TODO: consider a compile time flag to enable/disable the capture? returning
+  # from the block is unexpected behavior: the returned value won't be saved in
+  # the class variable.
+  def self.once(flag : Bool*, &)
+    Once.exec(flag) { yield } unless flag.value
   end
+end
 
+{% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %}
   # :nodoc:
+  #
+  # We always inline this accessor to optimize for the fast-path (already
+  # initialized).
   @[AlwaysInline]
-  fun __crystal_once(flag : Crystal::Once::State*, initializer : Void*)
-    return if flag.value.initialized?
+  fun __crystal_once(flag : Bool*, initializer : Void*)
+    return if flag.value
     Crystal.once(flag, initializer)
-    Intrinsics.unreachable unless flag.value.initialized?
-  end
-{% else %}
-  module Crystal
-    module Once
-      private def self.exec_impl(flag, &)
-        if flag.value
-          @@spin.unlock
-        elsif !wait_initializer?(flag)
-          run_initializer(flag) { yield }
-        end
-      end
-    end
 
-    # :nodoc:
-    def self.once(flag : Bool*, &)
-      Once.exec(flag) { yield } unless flag.value
-    end
+    # tells LLVM to assume that the flag is true, this avoids repeated access to
+    # the same constant or class variable to check the flag and try to run the
+    # initializer (only the first access will)
+    Intrinsics.unreachable unless flag.value
   end
-
+{% else %}
   # :nodoc:
+  #
+  # Unused. Kept for backward compatibility with older compilers.
   fun __crystal_once_init : Void*
     Pointer(Void).null
   end