[PATCH] D63321: [SVE][IR] Scalable Vector IR Type with pr42210 fix

Graham Hunter via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 03:17:25 PDT 2019


huntergr marked an inline comment as done.
huntergr added inline comments.


================
Comment at: lib/IR/Verifier.cpp:343
+          if (VTy->isScalable())
+            CheckFailed("Structs cannot contain scalable vectors", STy, &M);
+    }
----------------
thakis wrote:
> thakis wrote:
> > That's still a lot of type walking. Did you have a chance to measure time to thinlto-link some large-ish target with and without this change?
> Just saw your comment on https://bugs.llvm.org/show_bug.cgi?id=42210 . That seems fine to me. (I usually do min-of-3 when timing things, to reduce effects of caches.) So from my point of view this is ready for relanding.
Yeah, I ran them 5 times each (and cleared the thinlto cache directory between each run, though not the linux fs cache).

The main difference is that I'm not recursing into aggregates-within-aggregates; if there's aggregates with scalable types nested in arrays or structs, they'll still be picked up by the (mostly) linear walks. The fully recursive approach was a leftover from when we only rejected scalable vectors from global variables.


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

https://reviews.llvm.org/D63321





More information about the llvm-commits mailing list