compiler: Fix some real and theoretical miscompilations with allowzero and volatile
#21882
Conversation
|
cc @Rexicon226 for RISC-V bits. |
AstGen requires inline assembly to either have outputs or be marked volatile, so there doesn't appear to be any point in doing these checks.
These can all potentially operate on volatile pointers.
…inters. This informs optimization passes that they shouldn't assume that a load from a null pointer invokes undefined behavior. Closes ziglang#15816.
…spaces. LLVM considers null pointers to be valid for such address spaces.
Also fix some cases where we were being overzealous in applying volatile.
alexrp
force-pushed
the
compiler-fixes
branch
from
cf9f24f
to
8fd99f8
Compare
There was a problem hiding this comment.
The test coverage is far more important than the fixes. Please don't land this without full test coverage for the changed behavior.
Consider that this code would all change if some of these proposals being actively considered are implemented. Your fixes will be lost, however your test coverage would remain and be translated to the new language semantics, thus preserving your efforts here.
|
@andrewrk I'd love to add test coverage, but I'm not actually sure how. One possible way to test is to verify the generated IR, but do we have precedent for this? |
|
Ah, yes. There is no precedent for this, so it is nontrivial to add tests. Still important, though. Testing the generated machine code is certainly an option. That testing infrastructure would also be valuable for covering non-regression of optimizations. Another way to test |
I definitely agree with this, we'll need it for our own optimization framework anyways. |
|
So to be clear, most of the changes here are to the LLVM backend, with just a couple of necessary fixes to the self-hosted ones to get tests passing due to the AIR Testing for the LLVM backend does seem reasonable for this PR, though. But here I would argue we don't want to test the machine code output because that's going to change with every LLVM release (even patch releases). Similarly, verifying post-optimization LLVM IR would be prone to frequent changes. I think the right thing to do here is to verify the LLVM IR we generate, i.e. that we're conforming to the LLVM language reference. What happens from there is on LLVM.
Are there any existing VMs we can reuse for this, or would this involve writing our own test VM? It sounds useful for hard-to-test features like this in any case, and something I'd actually like to work on, but I think I'd like to do it as follow-up work and stick to verifying LLVM IR output for this PR. |
|
drive-by idea to test 'volatile': using ptrace and hardware debug registers to count mem accesses tried it (in C, because the also found this example that seem to support arm as well: |
That's a neat idea. I think if we want to test the run-time semantics, I would still prefer the WebAssembly VM idea just because it's more portable and the thing we're testing isn't actually target-specific. Can probably write a bare-bones VM in 1-2k lines of code, or even less if we limit it to just what we need for testing. We also need to be able to test |
Also some miscellaneous fixes while I was mucking about in the affected code.
Closes #15816.