[PATCH] D42244: [globalisel] Introduce LegalityQuery to better encapsulate the legalizer decisions. NFC.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/LegalizerInfo.h:273-276
   /// \returns a pair consisting of the kind of legalization that should be
   /// performed and the destination type.
-  std::pair<LegalizeAction, LLT> getAction(const InstrAspect &Aspect) const;
+  std::tuple<LegalizeAction, unsigned, LLT>
+  getAction(const LegalityQuery &Query) const;
----------------
bogner wrote:
> The bit about returning a pair in the doc isn't accurate anymore. 
> 
> Could we return a small pod struct with named fields instead of a tuple here? Tuples really don't have any advantage over simple structs in C++11 and later, IMO, and it's a lot harder to tell what's happening with them. I guess that would probably be a separate change, given that similar tuples are used elsewhere already.
> The bit about returning a pair in the doc isn't accurate anymore.

Good point. I'll fix that

> Could we return a small pod struct with named fields instead of a tuple here? Tuples really don't have any advantage over simple structs in C++11 and later, IMO, and it's a lot harder to tell what's happening with them. I guess that would probably be a separate change, given that similar tuples are used elsewhere already.

Sure. I went with a tuple since I was trying to bring it into line with the other getAction() below but it would be better to change both of them to a struct


https://reviews.llvm.org/D42244