[PATCH] D107902: [AArch64][SME] Disable NEON in streaming mode

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp:60
 
-  return createAArch64MCSubtargetInfoImpl(TT, CPU, /*TuneCPU*/ CPU, FS);
+  // Most of the NEON instruction set isn't supported in streaming mode on SME
+  // targets, but if no CPU is specified the default is "generic", which
----------------
c-rhodes wrote:
> paulwalker-arm wrote:
> > Does this need to be dependent on `CPU="Generic"`? I'm thinking you may still need to disable NEON even when targeting a known cpu.
> > 
> > I guess I expected some explicit opting in to the fact NEON will be disabled.  That's to say I figured the user would explicitly ask for `+streaming-sve` and by not also explicitly asking for `+neon` then they are making a conscious choice to disable NEON.
> > Does this need to be dependent on `CPU="Generic"`? I'm thinking you may still need to disable NEON even when targeting a known cpu.
> 
> CPUs with SME support will be defined without `FeatureNEON` in the features, so this won't be an issue?
> 
> > I guess I expected some explicit opting in to the fact NEON will be disabled.  That's to say I figured the user would explicitly ask for `+streaming-sve` and by not also explicitly asking for `+neon` then they are making a conscious choice to disable NEON.
> 
> That's what this change implements?
> > Does this need to be dependent on `CPU="Generic"`? I'm thinking you may still need to disable NEON even when targeting a known cpu.
> 
> CPUs with SME support will be defined without `FeatureNEON` in the features, so this won't be an issue?

This is incorrect, misunderstanding on my part. CPUs with SME support will also have NEON and as such FeatureNEON will be implied. 

> > I guess I expected some explicit opting in to the fact NEON will be disabled.  That's to say I figured the user would explicitly ask for `+streaming-sve` and by not also explicitly asking for `+neon` then they are making a conscious choice to disable NEON.
> 
> That's what this change implements?

I've updated the patch to disable NEON regardless of CPU if `+streaming-sve` is specified, unless NEON is explicitly requested.



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

https://reviews.llvm.org/D107902