[PATCH] D92219: [AMDGPU][GlobalISel] Avoid selecting S_PACK with constants

Tue Jan 5 10:30:20 PST 2021

foad added inline comments.

================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/Utils.h:146
+                                  bool HandleFConstants = true,
+                                  bool AnyExtAsZext = false);
 const ConstantFP* getConstantFPVRegVal(Register VReg,
----------------
paquette wrote:
> foad wrote:
> > mbrkusanin wrote:
> > > foad wrote:
> > > > I'm not sure if we need a flag for this. Could we do it unconditionally? @qcolombet ?
> > > There were no regressions for check-all when I tried it that way, but I wasn't sure if I should leave it like that.
> > I think it's probably safe to treat anyext the same as zext in this function, but maybe wait for comments from other globalisel maintainers (who may be on thanksgiving vacation).
> Maybe this should be `LookThroughAnyExt`? I imagine there are situations where we might want to treat G_ANYEXT as a sign extend.
> 
> e.g.
> 
> ```
> %cst:_(s32) = G_CONSTANT i32 -1
> %cst_wide:_(s64) = G_ANYEXT %cst
> ...
> %y:_(s64) = ...
> %x:_(s64) = G_ADD %y, %cst_wide
> ```
You mean, change the name of the argument to `LookThroughAnyExt`? But what would it do? Currently this function has to either sign- or zero-extend whatever constant it finds, so that it can return a value of the right bit width.

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

https://reviews.llvm.org/D92219