SILGen: "sort" the witness tables in declaration order #22308
Conversation
|
@swift-ci please test |
Emit the tables in the declaration order. This ensures that we get identical emission ordering across Windows and Linux. This imporves the test pass rate on Windows for the SILGen tests.
compnerd
force-pushed
the
noncanonical-is-more-canonical
branch
from
bb26f80
to
bb295c7
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@slavapestov ... dunno the exact details, but the address of the two decls is just different ( |
|
TypeDecl::compare should not depend on address order... |
|
It does do a pointer comparison, but, I can track through it again if it really matters (I think I agree with @gottesmm that emitting it in declaration order is nice) |
|
CC: @DougGregor |
|
Actually, I am curious about the need for the |
|
To be clear I think that is a question for @DougGregor (i.e. why do we need to sort). |
|
ping @DougGregor @jrose-apple - it's unclear the reason for the sorting, and seems to introduce a difference in the generated SILGen. |
|
I've got no context for this; we'll have to wait for Doug. |
|
@DougGregor ping |
|
There's no point in doing this sorting; the underlying data structures should be deterministic. I went a step further and killed the "sorted" parameter in #22659. Let's see if that solves the problem for good. |
|
@DougGregor - awesome, thanks! |
|
@DougGregor - I think I should look at that change more carefully at some point. That also caught the last two cases that I was trying to chase down! SILGen is now at 100% pass on Windows! |
Emit the tables in the declaration order. This ensures that we get identical
emission ordering across Windows and Linux. This imporves the test pass rate on
Windows for the SILGen tests.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.