[PATCH] D101725: [Verifier] Check that calls' arguments match the callee's byval/inalloca/preallocated

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 2 18:03:25 PDT 2021


dblaikie added a comment.

In D101725#2732577 <https://reviews.llvm.org/D101725#2732577>, @aeubanks wrote:

> I think the bitcode and textual representation both allow this issue.
> But based on the fact that so much of the checking for byval and related attributes is on the arguments rather than the parameters, IMO it's always been intended to have the attributes at least on the parameters. Users like Clang have always been properly emitting byval on the parameters (no check-clang tests regressed). This patch is just filling in a hole that should always have been filled in. But maybe someone else should chime in.

The argument is that this was always invalid, but a bug that it wasn't detected, so it doesn't need to be upgraded when reading IR, it's OK for that to fail the verifier)

Compared to an argument that it was valid, is now invalid, so needs to be upgraded to ensure bitcode compatibility?

Do you think it'd be especially complicated to auto-upgrade?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101725/new/

https://reviews.llvm.org/D101725



More information about the llvm-commits mailing list