Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable use of Labels as Values feature in Clang when not wrapped by GCC #97727

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

SlugFiller
Copy link
Contributor

Split off from #77233. Tested on top of #92316, using the benchmark:

@tool
extends EditorScript

func _run() -> void:
	static_run()

static func static_run() -> void:
	var time_before := Time.get_ticks_msec()

	var r : int
	for i in range(10000000):
		var next : int
		var first := 1
		var second := 2
		for c in range(40):
			next = first + second
			first = second
			second = next
		r = next
	print(r)

	var total_time := Time.get_ticks_msec() - time_before
	print("Time taken: " + str(total_time))

In dev build, no notable difference. But in release build with lto=full, times went down from 16-18 seconds, to 13 seconds, i.e. ~20% improvement in speed.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good.

Now that I see the changes closer again, IIUC, this only makes a different for distributions of Clang which do not define __GNUC__. I believe both Linux and Apple versions of Clang do define __GNUC__, so it's probably meaningful only for MSVC clang-cl and the vanilla Clang for Windows you're adding support for in the other PR?

Either way, this is correct and good to merge.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Oct 2, 2024
@akien-mga akien-mga merged commit 903c3bc into godotengine:master Oct 2, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants