[PATCH] D157813: [Driver][VE] Support VPU flag as a feature for VE

[Kazushi Marukawa via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Driver/ToolChains/Arch/VE.cpp:22
+                             std::vector<StringRef> &Features) {
+  // Defaults.
+  bool EnableVPU = true;
----------------
MaskRay wrote:
> kaz7 wrote:
> > MaskRay wrote:
> > > Delete the comment. The code speaks itself. 
> > > 
> > > ```
> > > if (Args.hasArg(options::OPT_mvevpu, options::OPT_mno_vevpu, true)
> > >   Features.push_back("+vpu");
> > > ```
> > Thanks.  I remove comments and simplify existing code.
> Sorry for my typo. We should use the 3-argument `hasFlag` here. It's simpler than getLastArg+matches
I see.  I was wondering that part too.  Now, I change it to use hasFlag function with default value.


================
Comment at: clang/test/Driver/ve-features.c:1
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mvevpu 2>&1 | FileCheck %s -check-prefix=VEVPU
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mno-vevpu 2>&1 | FileCheck %s -check-prefix=NO-VEVPU
----------------
kaz7 wrote:
> MaskRay wrote:
> > kaz7 wrote:
> > > MaskRay wrote:
> > > > `-target ` has been deprecated since Clang 3.4. Use `--target=`
> > > I didn't know that.  Thank you!
> > I think we typically spend just two RUN lines:
> > 
> > * one for the default
> > * one for `-mvevpu -mno-vevpu`
> > 
> > The additional coverage for having 3 RUN lines is probably not useful.
> Thank you for suggesting.  I consider what this patch should do and change the purpose to support VPU flag in the backend for VE.  As a result, Tests are two RUN lines.
> 
> - one for enable VPU
> - one for disable VPU
Because we use hasFlag with default value, test is changed again for following two RUN lines.
- one for the default
- one for `-mvevpu -mno-vevpu`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813