[PATCH] D32186: CodeGen: Add a hook for getFenceOperandTy

[Tony Tye via Phabricator via llvm-commits]

t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D32186#730749, @yaxunl wrote:

> In https://reviews.llvm.org/D32186#730742, @arsenm wrote:
>
> > Alternatively could these just be made i32 target constants for all targets?
>
>
> It requires changes lowering of ATOMIC_FENCE in several backends. I am not sure if those backends like this change.


I think making these constants makes more sense than inventing getFenceOperandTy(). It seems other targets are using the size of a pointer to address space 0 which does not seem right as these operands are for the memory scope and memory ordering which are not pointer types. Assuming that the size of a pointer to address space 0 is the size of size_t also seems incorrect. Since these values are in fact integer constants then doing as @arsenm suggests above seems the better approach.

However, that would require changing other backends, so this approach seems to be a work around to avoid that. So perhaps a TODO could be added that explains what the right fix is, and see if other backends would agree to the change in a later patch?

LGTM if add TODO.


https://reviews.llvm.org/D32186