Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

genx_volatile-attributed globals pointers can indeed be used by bitcast (constant expressions or in instruction form) before being used in genx.vload/vstore. #325

Open
lsatanov opened this issue Apr 12, 2024 · 0 comments

Comments

@lsatanov
Copy link
Contributor

lsatanov commented Apr 12, 2024

Hi, @KorovinVlad.

The commit
4f6d330
introduces checks for genx_volatile-attributed globals users.

One detail is that they can indeed be used by bitcast constant expression or instruction bitcasts before genx.vload/vstore.
E.g. pseudocode:

v = genx.vload(bitcast(@g*... to ...))

OR

ptr = bitcast(@g*... to ...)
v = genx.vload(ptr)

For this particular reason e.g. genx::getAVLoadSrcOrNull(...) routine will do the scan through bitcasts in reverse direction, up the def-use chain using getBitCastedValue(...): const auto *Src = getBitCastedValue(I->getOperand(0))

N.B. present LIT-tests do not include this kind of usage, but it can be found during real world workloads compilation, so, most probably it'd be useful to include those cases in the LIT test for GenXVerify.cpp.

Therefore here

const auto *I = dyn_cast<Instruction>(U);

genx::peelBitCastsWhileSingleUserChain(...) should be done before casting to an Instruction, like this:

auto InvalidUser = llvm::find_if(GV.users(), [](const User *U) {
  const auto *I = dyn_cast<Instruction>(genx::peelBitCastsWhileSingleUserChain(U));
  return !I || !(genx::isAVStore(I) || genx::isAVLoad(I));
});

Additionally, it would be also nice to see all of the wrong usages in IR, not only the first one, hence the code could look like this:

  auto IsInvalidUser = [](const User *U) {
    const auto *I = dyn_cast<Instruction>(genx::peelBitCastsWhileSingleUserChain(U));
    return !I || !(genx::isAVStore(I) || genx::isAVLoad(I));
  };

  for(const auto &U: GV.users())
    ensure(!IsInvalidUser(*U),
         "a global volatile variable may only be used in genx.vload/genx.vstore"
         "intrinsics or volatile load/store instructions, optionally preceeded"
         " by a pointer type bitcast",
         U);
@lsatanov lsatanov changed the title genx_volatile-attributed globals can indeed be used by bitcast constant expressions (in-call-embedded and standalone). genx_volatile-attributed globals can indeed be used by bitcast constant expressions (e.g. in-call-embedded). Apr 12, 2024
@lsatanov lsatanov changed the title genx_volatile-attributed globals can indeed be used by bitcast constant expressions (e.g. in-call-embedded). genx_volatile-attributed globals pointers can indeed be used by (bitcast constant expressions or in instruction form) before being used in genx.vload/vstore. Apr 12, 2024
@lsatanov lsatanov changed the title genx_volatile-attributed globals pointers can indeed be used by (bitcast constant expressions or in instruction form) before being used in genx.vload/vstore. genx_volatile-attributed globals pointers can indeed be used by bitcast (constant expressions or in instruction form) before being used in genx.vload/vstore. Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant