[PATCH] D81378: GlobalISel: Handle more cases in getGCDType

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 8 13:43:20 PDT 2020


arsenm marked 2 inline comments as done.
arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:551
+
+  if (OrigTy.isVector()) {
+    LLT OrigElt = OrigTy.getElementType();
----------------
aemerson wrote:
> Can we reorganize this to not have so much nesting?
> 
> Maybe duplicate `greatestCommonDivisor(OrigSize, TargetSize);` above this for the scalar case and early exit.
I'm not sure what you mean, the fallthroughs here are significant. I don't see where duplicating greatestCommonDivisor(OrigSize, TargetSize) would help.

I also wouldn't all anything here the scalar case; care is taken to manage the behavior of vector and "not vector" to preserve pointers where appropriate. I think isScalar not being the same as !isVector is a constant source of confusion


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:560
+      }
+    } else {
+      // If the source is a vector of pointers, return a pointer element.
----------------
aemerson wrote:
> Invert this so the else early exits?
These don't return though? It falls through


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

https://reviews.llvm.org/D81378





More information about the llvm-commits mailing list