Page MenuHomePhabricator

[PATCH 05/38] [noalias] [IR] Introduce noalias_sidechannel for LoadInst/StoreInst
Needs ReviewPublic

Authored by jeroen.dobbelaere on Oct 4 2019, 2:17 PM.

Details

Reviewers
hfinkel
jdoerfert
Summary

This is part of the series started by D68484.

The noalias_sidechannel tracks dependencies on noalias intrinsics without
blocking optimizations on the pointer value.

Note: this implementation might have some speed effect on all instruction
because of the extra bit that is used to track if the noalias_sidechannel is
present or not. This might impact the computation of the number of arguments.

Note: this is a stable point and tests should run fine with the patches applied up to this point.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2019, 2:17 PM
jeroen.dobbelaere edited the summary of this revision. (Show Details)Oct 4 2019, 2:19 PM

All that subtle magic about "we may have 2 operands but in some places we need to pretend that we have 3 operands" seems suboptimal to me.
Semantically, can we just always allocate space for 3 operands, but use either nullptr or undef as a magic to deduce whether we actually have the third operand?

llvm/include/llvm/IR/Instructions.h
313

assertion message missing

471

assertion message missing

All that subtle magic about "we may have 2 operands but in some places we need to pretend that we have 3 operands" seems suboptimal to me.
Semantically, can we just always allocate space for 3 operands, but use either nullptr or undef as a magic to deduce whether we actually have the third operand?

It is kind of magic and it took some time to get it right. The technique comes from GlobalVariable where there is either 0 or 1 operands.
There are 2 operands allocated for LoadInst, 3 for StoreInst. The challenge is that the operands are allocated before the instruction.
When the NumUserOperands changes, suddenly all operands are shifted. That is not convenient. That's why I choose to keep NumUserOperands constant, and to
add an extra 'delta'. Any better solution that works is welcome though.

Added assert message; clang-format

jeroen.dobbelaere marked 2 inline comments as done.Oct 7 2019, 7:47 AM
a.elovikov added inline comments.
llvm/lib/IR/Instructions.cpp
4150

Why is it safe? Consider we have aliasing load and store, we clone them so load.clone and load.store now have undef in the sidechannels, so optimizations are free to choose the values for them that would mean noalias.

jeroen.dobbelaere marked an inline comment as done.Oct 10 2019, 1:49 PM
jeroen.dobbelaere added inline comments.
llvm/lib/IR/Instructions.cpp
4150
  • Undef is maybe the wrong value and could indeed trigger problems in future.
  • null is not safe, as that is used to indicate a converted load/store instruction, when it does not depend on a restrict pointer (still in combination with the !noalias metadata, which must be present and indicate what scopes are visible)..
  • Copying the noalias_sidechannel from the original is also not safe. It might not be valid in the new context.
  • Removing the noalias_sidechannel (as was done in some earlier version) also does not work: some passes rightfully expect the clone to have the same number of arguments as the original.