Skip to content

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Aug 20, 2025

follows up #24818
relates to #23923

fixes #25109
fixes #25111

transform addr ( conv ( x ) ) -> conv ( addr ( x ) ) so that it is the original value that is being modified

T1_ = ((unsigned long long*) ((&a_1)));
r(T1_);

@ringabout ringabout changed the title fixes #25109; fixes #25111 transform addr ( conv ( x ) ) -> conv ( addr ( x ) ) fixes #25109; fixes #25111 transform addr(conv(x)) -> conv(addr(x)) Aug 20, 2025
@Araq Araq merged commit b527db9 into devel Aug 21, 2025
23 of 27 checks passed
@Araq Araq deleted the pr_sozusagen branch August 21, 2025 11:31
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from b527db9

Hint: mm: orc; opt: speed; options: -d:release
182980 lines; 8.560s; 658.121MiB peakmem

@arnetheduck
Copy link
Contributor

This transformation (still) looks like it will violate strict aliasing, similar to #24818 (comment) ..

@arnetheduck
Copy link
Contributor

ie maybe on the nim side, this is fine, but it's not a valid transformation to make in C - even if two integers have the same byte size, they are not the same type and therefore may not alias - instead, one has to either union or memcpy values

@ringabout
Copy link
Member Author

ringabout commented Aug 21, 2025

void main(NU64* v) {
  unsigned long long tmp = (unsigned long long)(*v);
  foo(&tmp);
  *v = tmp
}

This seems to much harder to be transformed into. Perhaps I can improve it with memcpy for aliasing

@Araq
Copy link
Member

Araq commented Aug 21, 2025

We don't care about aliasing though, it's impossible to use properly for a code generator (and also for most mortal beings).

@arnetheduck
Copy link
Contributor

We don't care about aliasing though

the C optimiser does though - and in this particular case, there exists a translation that is both correct, alias-free and fairly simple - it just requires using a different technique than casting on the C side.

@bptato
Copy link
Contributor

bptato commented Aug 21, 2025

We don't care about aliasing though

Why not apply #25067 then?

@Araq
Copy link
Member

Araq commented Aug 21, 2025

the C optimiser does though - and in this particular case, there exists a translation that is both correct, alias-free and fairly simple - it just requires using a different technique than casting on the C side.

Maybe but I think it's just a whack-a-mole game. I also think the C compilers themselves cannot really follow C's aliasing rules as too much realworld C code would simply break...

@arnetheduck
Copy link
Contributor

whack-a-mole game.

It's just a set of rules - they're fairly consistent and can be summarized as "copy, don't cast when in doubt" - and yes, optimizers do use them to optimize code, and more aggressively so when you enable LTO - Nim is in particular affected due to how nim spreads implementations all over the place with generics - but in return, you get a 20-30% performance improvement, for simply following the language rules. #24596 is a real-world example.

narimiran pushed a commit that referenced this pull request Aug 23, 2025
…)` (#25112)

follows up #24818
relates to #23923

fixes #25109
fixes #25111

transform `addr ( conv ( x ) )` -> `conv ( addr ( x ) )` so that it is
the original value that is being modified

```c
T1_ = ((unsigned long long*) ((&a_1)));
r(T1_);
```

(cherry picked from commit b527db9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants