[PATCH] D42251: [globalisel][legalizer] Adapt LegalizerInfo to support inter-type dependencies and other things.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/LegalizerInfo.h:348
+        },
+        LegalizeMutations::pick(TypeIdx, Ty));
+  }
----------------
dsanders wrote:
> bogner wrote:
> > dsanders wrote:
> > > reames wrote:
> > > > You might want to choose a different verb than "pick" here.  Or at least, it isn't obvious from just reading the code what this does.
> > > One of my colleagues has said the same thing in person a couple days ago but I haven't come up with anything better yet. The intent is that it returns a mutation function that always returns the given typeidx and type.
> > I'd probably just go with "identity". It's not great, but I think it's clearer/more precise than pick.
> Ok, I'll go with identity for now but we ought to come up with a better name still. `identity()` makes it sound like you get a mutator that doesn't change anything but that's not correct. You get a mutator that changes the type to a specific type.
> 
> For example, given:
>    LegalityQuery({G_ADD, {s32})
> the rule:
>   .widenScalarIf(typeInSet(0, {s32}), identity(0, s64))
> will result in the action:
>   {WidenScalar, /*TypeIdx*/0, s64}
> which will widen the G_ADD to work on s64's.
I'm not sure why this didn't occur to me until after I committed but `changeTo` would be a better name than either pick or identity.


Repository:
  rL LLVM

https://reviews.llvm.org/D42251