[PATCH] D73286: [llvm][VectorUtils] Tweak VFShape for scalable vector functions.

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 28 09:51:51 PST 2020


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


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:472
+
+  assert(VF && "VF must be a positive value.");
   const VFShape Shape({VF, IsScalable, Parameters});
----------------
andwar wrote:
> This should never be hit because for `VF = 0` `tryParseVLEN ` returns an error and `getECFromSignature` returns at least 1.
> 
> I'm just wondering, maybe less would be more here?
> 
> 
> 
> This should never be hit because for VF = 0 tryParseVLEN returns an error and getECFromSignature returns at least 1.

yes, but...

> I'm just wondering, maybe less would be more here?

... I'd rather leave the assert to make sure that if someone adds code after the invocation of `tryParseVLEN` that sets `VF` to zero, they get a failure to make them aware of the problem.

Alternatively, we could make `VF` in VFShape a class that can only be a strictly positive number... but that seems to be over-killing... 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73286





More information about the llvm-commits mailing list