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

Excessive if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_; generation #23311

Closed
guzba opened this issue Feb 16, 2024 · 8 comments
Closed

Excessive if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_; generation #23311

guzba opened this issue Feb 16, 2024 · 8 comments

Comments

@guzba
Copy link
Contributor

guzba commented Feb 16, 2024

Description

I recently did some performance-related work and noticed that a lot of code is generated related to exceptions when looking at the C. This excessive checking seems unavoidable as a compiler user and is generated even in what appears to me to be no-point-situations.

Compiled with nim c --mm:arc -d:release -rf

Example code:

proc procA*(arg: int): bool {.inline, raises: [].} =
  arg < 0

proc procB*(arg: int): bool {.inline, raises: [].} =
  arg > 0

proc procC*(arg: int): bool {.inline, raises: [].} =
  arg.procA() or arg.procB()

var i = 3
echo procC(i)

Generates:

static N_INLINE(NIM_BOOL, procC__raisetheroof_u7)(NI arg_p0) {
	NIM_BOOL result;
	NIM_BOOL T1_;
NIM_BOOL* nimErr_;
{nimErr_ = nimErrorFlag();
	T1_ = (NIM_BOOL)0;
	T1_ = procA__raisetheroof_u1(arg_p0);
	if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_;
	if (T1_) goto LA2_;
	T1_ = procB__raisetheroof_u4(arg_p0);
	if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_;
LA2_: ;
	result = T1_;
	}BeforeRet_: ;
	return result;
}

So these inline and raises: [] procs still resulted in the if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_; check. Maybe this check is unavoidable in all cases?

This is just a simplified example. The number of checks is very significant as the the amount of Nim grows.

This is relevant because when doing this performance work I found that ~25% of my program's runtime was at tlv_get_addr according to macOS Instruments (noticed this when working on Mac). Perhaps these things are not related, but extern NIM_THREADVAR NIM_BOOL nimInErrorMode__system_u4315; is the only threadvar in the generated C code and then follows:

static N_INLINE(NIM_BOOL*, nimErrorFlag)(void) {
	NIM_BOOL* result;
	result = (&nimInErrorMode__system_u4315);
	return result;
}

and

nimErr_ = nimErrorFlag();

Am I looking in the wrong place? Is this checking unavoidable and raises: [] can't help? Should these checks be getting optimized out by the C compiler? Thanks for your time.

Nim Version

Nim Compiler Version 2.0.2 [Windows: amd64]
Compiled at 2023-12-15
Copyright (c) 2006-2023 by Andreas Rumpf

active boot switches: -d:release

Current Output

No response

Expected Output

No response

Possible Solution

No response

Additional Information

No response

@guzba
Copy link
Contributor Author

guzba commented Feb 16, 2024

To add a quick note, I tried nim cpp instead of nim c and got a better result, both much faster performance and less error-check-ey generated code for the current stuff I'm working on that caused me to ask this question.

@Araq
Copy link
Member

Araq commented Feb 16, 2024

If tlv_get_addr shows up in your profile it indicates that your platform has a suboptimal thread local storage implementation.

clang can optimize out these accesses but I don't know when it does it. We should investigate. :-)

As for your questions: Nim is pessimistic with raises: [] because IndexError etc. are recoverable. There is a .quirky annotation that you can use to disable the checks for specific procs and you can also .push it iirc. .quirky needs Nim devel though I think.

@guzba
Copy link
Contributor Author

guzba commented Feb 16, 2024

@Araq thanks for the reply. I was not aware of the quirky pragma. Using quirky appears to be working as expected when compiling with the latest stable (2.0.2). I have only just started to test it out but so far so good.

Does quirky have implications with nim c (which uses --exceptions:goto) other than omitting error checking?

I am very happy be able to have some control over when the error checking is generated! I can now experiment with what (if any) cost various levels of checking has instead of just reaching a wall.

Now that I understand why the checks are generated (pessimistic as explained makes sense), I am fully satisfied by having the option to simply do a little more work to get the results I want in the specific cases it matters.

As for tlv_get_addr and seeing if clang can optimize out the accesses, I'll need to come back to that with more data and experimentation. If I have interesting / useful findings I'll open a new more focused issue.

@Araq
Copy link
Member

Araq commented Feb 17, 2024

Does quirky have implications with nim c (which uses --exceptions:goto) other than omitting error checking?

Well no but "omitting error checking" can lead to code that is tricky to get right when you use exception handling for control flow. Been there, done that. ;-)

@treeform
Copy link
Contributor

I think the main thing Guzba and I are trying to achieve is the ability to profile our code to identify hotspots. We think a lot about hotspots and rewrite them in a more "C-like" manner, where we really consider allocations, branch predictions, cache evictions, and maybe sprinkle in some SIMD. We know everything that can happen to this code because we've thought about it and have tests, fuzzing tests, and benchmarks. We just want to be able to mark some parts as "optimized code here: super unsafe but highly tested." We want Nim to generate plain C without any smart features. Does setting checks: off and using quirky allow us to do that?

@zerbina
Copy link
Contributor

zerbina commented Feb 17, 2024

To expand on what Araq said, the Nim compiler doesn't track where Defect are raised, and since they're, by default, catchable, it has to assume that every procedure (except for some built-ins) raise a Defect, even if they in practice do not.

The quirky option force-disables most usage of the error flag, in effect meaning that, within a quiry procedure, it appears as if all called procedures have no exceptional exit; using any try statements/expressions (both explicit and implicit) in a quirky procedure will net you C compiler errors.

We want Nim to generate plain C without any smart features. Does setting checks: off and using quirky allow us to do that?

checks:off disables all compiler-inserted run-time checks, such as integer overflow, index, bound, range, and field checks.

If you want exceptions to continue to work as they normally do, but still get rid of the pessimistic unwind handling, you can use --panics:on (can only be set program-wide).

When panics are enabled, all defects raised by compiler-inserted run-time checks immediately terminate the program with quit, no unwinding takes place for them. In effect, this means that Defects raised by compiler-inserted run-time checks cannot be caught or handled in any way. This allows the compiler to omit the unwind handling (if (NIM_UNLIKELY(*nimErr)) goto ...) for all calls to procedures that are inferred as .raises: [].

@Araq
Copy link
Member

Araq commented Feb 17, 2024

Does setting checks: off and using quirky allow us to do that?

Yes.

@guzba
Copy link
Contributor Author

guzba commented Feb 19, 2024

Thanks for the helpful replies here!

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

No branches or pull requests

4 participants