Skip to content

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Feb 18, 2025

This reverts a portion of commit 50833c8.

This algorithm is not able to handle simple cases where there is any internal padding, such as the example of:

struct LotsBytes
    a::Int8
    b::NTuple{256,Int}
    c::Int
end

Unfortunately fixing it is a bit of a large project right now, so reverting now to fix correctness while working on that.

Fixes #55513 (indirectly, by removing broken code) Maybe reopens #54109, although the latency issue it proposes to fix doesn't occur on master even with this revert (just the mediocre looking IR result output returns)

This reverts a portion of commit 50833c8.

This algorithm is not able to handle simple cases where there is any
internal padding, such as the example of:
```
struct LotsBytes
    a::Int8
    b::NTuple{256,Int}
    c::Int
end
```

Unfortunately fixing it is a bit of a large project right now, so
reverting now to fix correctness while working on that.

Fixes #55513 (indirectly, by removing broken code)
Maybe reopens #54109, although the latency issue it proposes to fix
doesn't occur on master even with this revert (just the mediocre looking
IR result output returns)
@vtjnash vtjnash added backport 1.12 Change should be backported to release-1.12 compiler:codegen Generation of LLVM IR and native code labels Feb 18, 2025
@KristofferC
Copy link
Member

This should also add a test with that struct?

@vtjnash
Copy link
Member Author

vtjnash commented Feb 18, 2025

I've deleted the code that makes that a special case, so it falls back to being covered by existing tests

@vtjnash vtjnash merged commit a65c2cf into master Feb 19, 2025
7 of 9 checks passed
@vtjnash vtjnash deleted the jn/54121-revert branch February 19, 2025 15:30
@oscardssmith oscardssmith restored the jn/54121-revert branch February 19, 2025 19:30
KristofferC pushed a commit that referenced this pull request Feb 21, 2025
This reverts a portion of commit
50833c8.

This algorithm is not able to handle simple cases where there is any
internal padding, such as the example of:
```
struct LotsBytes
    a::Int8
    b::NTuple{256,Int}
    c::Int
end
```

Unfortunately fixing it is a bit of a large project right now, so
reverting now to fix correctness while working on that.

Fixes #55513 (indirectly, by removing broken code) Maybe reopens #54109,
although the latency issue it proposes to fix doesn't occur on master
even with this revert (just the mediocre looking IR result output
returns)

(cherry picked from commit a65c2cf)
@KristofferC KristofferC mentioned this pull request Feb 21, 2025
24 tasks
KristofferC added a commit that referenced this pull request Feb 26, 2025
Backported PRs:
- [x] #57302 <!-- Add explicit imports for types and fix bugs -->
- [x] #57420 <!-- Compiler: Fix check for IRShow definedness -->
- [x] #57419 <!-- generated: Switch resolution module back to what it
was before -->
- [x] #57421 <!-- bpart: Skip implicit import reval if using'd export
set is unchanged -->
- [x] #57425 <!-- Prohibit binding replacement in closed modules during
precompile -->
- [x] #57426 <!-- Prohibit `import`ing or `using` Main during
incremental compilation -->
- [x] #57433 <!-- bpart: Track whether any binding replacement has
happened in image modules -->
- [x] #57445 <!-- Run all `--sysimage-native-code=no` cmdlineargs tests
single-threaded -->
- [x] #57386 <!-- Only strip invariant.load from special pointers -->
- [x] #57453 <!-- Revert "Make emitted egal code more loopy (#54121)"
-->
- [x] #57389 <!-- Change memory indexing to use the type as index
instead of i8 -->
- [x] #57447 <!-- Don't return null pointer when asking for the type of
a declared global -->
- [x] #57467 <!-- using/import: ensure world update after each
observable operation -->
- [x] #57471 <!-- staticdata: Don't use `newm` pointer after it has been
invalidated -->
- [x] #57416 <!-- lowering: Don't mutate lambda in `linearize` -->
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 26, 2025
serenity4 pushed a commit to serenity4/julia that referenced this pull request May 1, 2025
…ng#57453)

This reverts a portion of commit
50833c8.

This algorithm is not able to handle simple cases where there is any
internal padding, such as the example of:
```
struct LotsBytes
    a::Int8
    b::NTuple{256,Int}
    c::Int
end
```

Unfortunately fixing it is a bit of a large project right now, so
reverting now to fix correctness while working on that.

Fixes JuliaLang#55513 (indirectly, by removing broken code) Maybe reopens JuliaLang#54109,
although the latency issue it proposes to fix doesn't occur on master
even with this revert (just the mediocre looking IR result output
returns)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RetentionParameterEstimator hits incorrect assert in emit_masked_bits_compare
2 participants