[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

The description should explain why this change is necessary.




Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:1
+// RUN: %clang -mllvm -enable-ppc-gen-scalar-mass -O3 -fapprox-func 
--target=powerpc64le-unknown-linux-gnu -S %s -o -| FileCheck %s 
-check-prefix=CHECK-AFN
+// RUN: %clang -mllvm -enable-ppc-gen-scalar-mass -Ofast 
--target=powerpc64le-unknown-linux-gnu -S %s -o -| FileCheck %s 
-check-prefix=CHECK-FAST

Please add negative end-to-end tests as well (eg. when 
`-enable-ppc-gen-scalar-mass` is false or when `-fno-approx-func`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:33-34
+// CHECK-MASS-AFN: __xl_sin
+// CHECK-NOMASS-FAST-NOT: __xl_sin_finite
+// CHECK-NOMASS-AFN-NOT: __xl_sin
+// CHECK: blr

Please also check for

CHECK-NOMASS-FAST-NOT: __xl_sin
CHECK-NOMASS-AFN-NOT: __xl_sin_finit

and do the same for other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:33
+// CHECK-MASS-AFN: __xl_sin
+// CHECK-NO-MASS-FAST-NOT: __xl_sin{{_finite}}
+// CHECK-NO-MASS-AFN-NOT: __xl_sin{{_finite}}

I don't think this works the way you expect it:

```
> cat input.txt
; CHECK-NOT: __xl_sin{{_finite}}
> echo "__xl_sin" | ./bin/FileCheck ./input.txt
>
```

this seems closer to what you want:
```
; CHECK-NOT: {{__xl_sin|__xl_sin_finite}}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour accepted this revision.
bmahjour added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1726
   NegFlag>;
-def fapprox_func : Flag<["-"], "fapprox-func">, Group, 
Flags<[CC1Option, NoDriverOption]>,
-  MarshallingInfoFlag>, 
ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>;
+defm approx_func : BoolFOption<"approx-func", LangOpts<"ApproxFunc">, 
DefaultFalse,
+  PosFlag, 
NegFlag>;

I think we should separate out the clang driver interface into its own patch 
and ask for feedback on the mailing list. One key question would be, should 
this option assume no-errno and no-trapping-math or not (given that there is no 
IR representation for them).

There should also be LIT tests dedicated to this to verify the clang interface. 
I only see llc interface being tested in this patch.



Comment at: llvm/include/llvm/Target/TargetOptions.h:179
+/// with approximate calculations
+unsigned ApproxFuncFPMath : 1;
+

We already have the `PPCGenScalarMASSEntries` bit, why do we need another one? 
Perhaps we can remove `PPCGenScalarMASSEntries`, but we should not have to turn 
on two options to get one transformation enabled.



Comment at: llvm/lib/Target/PowerPC/PPCGenScalarMASSEntries.cpp:72
+  // checking fast flag insread of nnan/ninf/afn because errno and 
+  // trapping-math don't have IR representation.
+  return CI.isFast();

...but errno and trapping-math would be an issue for non-finite entries as well.

Again, I think this function should just check for nnan/ninf/afn flags. We need 
to find out (with the help of the wider community) how to deal with the 
concerns surrounding errno and traps separately. One way to do that would be to 
broaden the definition of the `afn` flag to include no-errno and no-trapping 
semantics. Another way might be to make clang FE set the `afn` bit only if 
`-fno-math-errno` and `-fno-trapping-math` options are enabled (less 
desirable). A third way might be to add corresponding  function attributes to 
the IR for `-fno-math-errno` and `-fno-trapping-math`.

Once these issues are sorted out, we can add the appropriate constraints to the 
`isCandidateSafeToLower` function.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1370
+  // to be consistent to PPCGenScalarMASSEntries pass
+  if (TM.Options.PPCGenScalarMASSEntries && TM.Options.ApproxFuncFPMath) {
+if (TM.Options.NoInfsFPMath && TM.Options.NoNaNsFPMath &&

if someone compiles with -Ofast without any extra options, would 
`TM.Options.ApproxFuncFPMath` be true here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106409: [PowerPC] Add diagnostic for out of range values for vec_cts,vec_ctf

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

Thanks for this patch @ZarkoCA.

We have six of these functions in altivec: `vec_ctd, vec_ctf, vec_cts, 
vec_ctsl, vec_ctu, vec_ctul`. They are all defined as macros in altivec.h and 
not all of them map to the builtins checked in this patch (eg `vec_ctul`). I 
realize this is an improvement over what we have, but just wondering if you've 
considered diagnosing cases like `vec_ctul` or the following:

  vector double a; 
  vector unsigned int res = vec_ctu(a, 32);

One solution I can think of is to introduce a noop pass-through builtin whose 
only purpose would be to allow semantic checking in places like 
`Sema::CheckPPCBuiltinFunctionCall`. For example say the builtin is called 
`_builtin_pass_through_with_rcheck_1_32` then we can change `vec_ctul` in 
altivec.h to the following:

  #define vec_ctul(__a, __b)
 \
_Generic((__a), vector float
 \
 : __extension__({  
 \
 vector float __ret =   
 \
 (vector float)(__a) *  
 \
 (vector float)(vector unsigned)((0x7f + 
(__builtin_pass_through_with_rcheck_1_32(__b))) << 23);  \
 __builtin_vsx_xvcvspuxds(  
 \
 __builtin_vsx_xxsldwi(__ret, __ret, 1));   
 \
   }),  
 \

and the sema check would look something like:

  case PPC::BI__builtin_pass_through_with_rcheck_1_32:
  return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31);

Any thoughts @nemanjai ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106409

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106409: [PowerPC] Add diagnostic for out of range values for vec_cts,vec_ctf

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D106409#2893592 , @nemanjai wrote:

> My preference would be to just truncate the value with `& 0x1F`. It won't 
> produce any code in the binary and will work as expected.

That's fine with me too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106409

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour updated this revision to Diff 310607.
bmahjour marked 5 inline comments as done.
bmahjour added a comment.
Herald added subscribers: cfe-commits, dexonsmith, ecnelises, martong, 
javed.absar, MatzeB.
Herald added a project: clang.

Thanks for the review @Meinersbur and sorry for taking so long to address your 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  llvm/include/llvm/Analysis/CFGPrinter.h
  llvm/include/llvm/Analysis/DDG.h
  llvm/include/llvm/Analysis/DDGPrinter.h
  llvm/include/llvm/Support/DOTGraphTraits.h
  llvm/include/llvm/Support/GraphWriter.h
  llvm/lib/Analysis/CFGPrinter.cpp
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/CallPrinter.cpp
  llvm/lib/Analysis/DDGPrinter.cpp
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def

Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -384,6 +384,7 @@
 #define LOOP_PASS(NAME, CREATE_PASS)
 #endif
 LOOP_PASS("canon-freeze", CanonicalizeFreezeInLoopsPass())
+LOOP_PASS("dot-ddg", DDGDotPrinterPass())
 LOOP_PASS("invalidate", InvalidateAllAnalysesPass())
 LOOP_PASS("licm", LICMPass())
 LOOP_PASS("loop-idiom", LoopIdiomRecognizePass())
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -29,6 +29,7 @@
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/DDG.h"
+#include "llvm/Analysis/DDGPrinter.h"
 #include "llvm/Analysis/Delinearization.h"
 #include "llvm/Analysis/DemandedBits.h"
 #include "llvm/Analysis/DependenceAnalysis.h"
Index: llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
===
--- llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
+++ llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
@@ -35,7 +35,7 @@
   return true;
 }
 
-static bool isNodeHidden(const SUnit *Node) {
+static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
   return (Node->NumPreds > 10 || Node->NumSuccs > 10);
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -3836,7 +3836,7 @@
 return true;
   }
 
-  static bool isNodeHidden(const SUnit *Node) {
+  static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
 if (ViewMISchedCutoff == 0)
   return false;
 return (Node->Preds.size() > ViewMISchedCutoff
Index: llvm/lib/Analysis/DDGPrinter.cpp
===
--- /dev/null
+++ llvm/lib/Analysis/DDGPrinter.cpp
@@ -0,0 +1,150 @@
+//===- DDGPrinter.cpp - DOT printer for the data dependence graph --==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+//
+// This file defines the `-dot-ddg` analysis pass, which emits DDG in DOT format
+// in a file named `ddg..dot` for each loop  in a function.
+//===--===//
+
+#include "llvm/Analysis/DDGPrinter.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/GraphWriter.h"
+
+using namespace llvm;
+
+static cl::opt DotOnly("dot-ddg-only", cl::init(false), cl::Hidden,
+ cl::ZeroOrMore, cl::desc("simple ddg dot graph"));
+static cl::opt DDGDotFilenamePrefix(
+"dot-ddg-filename-prefix", cl::init("ddg"), cl::Hidden,
+cl::desc("The prefix used for the DDG dot file names."));
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly = false);
+
+//======//
+// Implementation of DDG DOT Printer for a loop
+//======//
+PreservedAnalyses DDGDotPrinterPass::run(Loop &L, LoopAnalysisManager &AM,
+ LoopStandardAnalysisResults &AR,
+ LPMUpdater &U) {
+  writeDDGToDotFile(*AM.getResult(L, AR), DotOnly);
+  return PreservedAnalyses::all();
+}
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly) {
+  std::string Filename =
+  Twine(DDGDotFilenamePrefix + "." + G.getName() + ".dot").str();
+  errs()

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: llvm/include/llvm/Analysis/DDG.h:480
+return OS.str();
+  unsigned count = 0;
+  for (auto &D : Deps) {

Meinersbur wrote:
> [suggestion] Instead of count, you can use `llvm::enumerate`. I think it's 
> nicer to check at for "not the first" at the beginning instead of the last 
> element at the end.
> 
> There also is an `interleave` function in STLExtras.h for uses cases like 
> this.
Interesting! Thanks for pointing out those utilities! I'll use interleave.



Comment at: llvm/include/llvm/Analysis/DDGPrinter.h:45
+assert(G && "expected a valid pointer to the graph.");
+Graph = G;
+return "DDG for '" + std::string(G->getName()) + "'";

Meinersbur wrote:
> Why does a getter set a field value?
Good question :)

When printing pi-blocks we would like to show the member nodes being enclosed 
by the pi-node. However, since the member nodes are also part of the graph, 
they will get visited during the walk and get dumped into the resulting dot 
file. To solve this, I'm trying to hide the member nodes from the walk (via 
`isNodeHidden()`) when they get visited and then print them as part of printing 
the pi-block node. 

The problem I encountered was that, `isNodeHidden()` only takes a graph node as 
a parameter, but for me to know whether a node belongs to a pi-block I need to 
get access to the graph that contains the nodes. To get around this, I'm 
caching a pointer to the graph when the `getGraphName` gets called and use it 
when `isNodeHidden` is invokedyikes!

A cleaner solution (and one that may have other uses in the future) is to make 
`isNodeHidden()` take a graph object as argument (similar to `getNodeLabel()`, 
and `getGraphName()`). I'll update the patch to include that change.



Comment at: llvm/lib/Analysis/DDGPrinter.cpp:109
+  if (isa(Node))
+for (auto *II : static_cast(Node)->getInstructions())
+  OS << *II << "\n";

Meinersbur wrote:
> Does this const_cast exist to add const qualifier?
No. The static_cast is used because the `getInstructions()` is a member 
function of `SimpleDDGNode` but not `DDGNode`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour updated this revision to Diff 311678.
bmahjour added a comment.

fix formatting and use interleaveComma instead of interleave.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  llvm/include/llvm/Analysis/CFGPrinter.h
  llvm/include/llvm/Analysis/DDG.h
  llvm/include/llvm/Analysis/DDGPrinter.h
  llvm/include/llvm/Support/DOTGraphTraits.h
  llvm/include/llvm/Support/GraphWriter.h
  llvm/lib/Analysis/CFGPrinter.cpp
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/CallPrinter.cpp
  llvm/lib/Analysis/DDGPrinter.cpp
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def

Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -384,6 +384,7 @@
 #define LOOP_PASS(NAME, CREATE_PASS)
 #endif
 LOOP_PASS("canon-freeze", CanonicalizeFreezeInLoopsPass())
+LOOP_PASS("dot-ddg", DDGDotPrinterPass())
 LOOP_PASS("invalidate", InvalidateAllAnalysesPass())
 LOOP_PASS("licm", LICMPass())
 LOOP_PASS("loop-idiom", LoopIdiomRecognizePass())
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -29,6 +29,7 @@
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/DDG.h"
+#include "llvm/Analysis/DDGPrinter.h"
 #include "llvm/Analysis/Delinearization.h"
 #include "llvm/Analysis/DemandedBits.h"
 #include "llvm/Analysis/DependenceAnalysis.h"
Index: llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
===
--- llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
+++ llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
@@ -35,7 +35,7 @@
   return true;
 }
 
-static bool isNodeHidden(const SUnit *Node) {
+static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
   return (Node->NumPreds > 10 || Node->NumSuccs > 10);
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -3836,7 +3836,7 @@
 return true;
   }
 
-  static bool isNodeHidden(const SUnit *Node) {
+  static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
 if (ViewMISchedCutoff == 0)
   return false;
 return (Node->Preds.size() > ViewMISchedCutoff
Index: llvm/lib/Analysis/DDGPrinter.cpp
===
--- /dev/null
+++ llvm/lib/Analysis/DDGPrinter.cpp
@@ -0,0 +1,150 @@
+//===- DDGPrinter.cpp - DOT printer for the data dependence graph --==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+//
+// This file defines the `-dot-ddg` analysis pass, which emits DDG in DOT format
+// in a file named `ddg..dot` for each loop  in a function.
+//===--===//
+
+#include "llvm/Analysis/DDGPrinter.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/GraphWriter.h"
+
+using namespace llvm;
+
+static cl::opt DotOnly("dot-ddg-only", cl::init(false), cl::Hidden,
+ cl::ZeroOrMore, cl::desc("simple ddg dot graph"));
+static cl::opt DDGDotFilenamePrefix(
+"dot-ddg-filename-prefix", cl::init("ddg"), cl::Hidden,
+cl::desc("The prefix used for the DDG dot file names."));
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly = false);
+
+//======//
+// Implementation of DDG DOT Printer for a loop
+//======//
+PreservedAnalyses DDGDotPrinterPass::run(Loop &L, LoopAnalysisManager &AM,
+ LoopStandardAnalysisResults &AR,
+ LPMUpdater &U) {
+  writeDDGToDotFile(*AM.getResult(L, AR), DotOnly);
+  return PreservedAnalyses::all();
+}
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly) {
+  std::string Filename =
+  Twine(DDGDotFilenamePrefix + "." + G.getName() + ".dot").str();
+  errs() << "Writing '" << Filename << "'...";
+
+  std::error_code EC;
+  raw_fd_ostream File(Filename, EC, sys::fs::F_Text);
+
+  if (!EC)
+// We only provide the constant verson of the DOTGraphTra

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd4a10732c8b: [DDG] Data Dependence Graph - DOT printer 
(authored by bmahjour).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  llvm/include/llvm/Analysis/CFGPrinter.h
  llvm/include/llvm/Analysis/DDG.h
  llvm/include/llvm/Analysis/DDGPrinter.h
  llvm/include/llvm/Support/DOTGraphTraits.h
  llvm/include/llvm/Support/GraphWriter.h
  llvm/lib/Analysis/CFGPrinter.cpp
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/CallPrinter.cpp
  llvm/lib/Analysis/DDGPrinter.cpp
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def

Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -384,6 +384,7 @@
 #define LOOP_PASS(NAME, CREATE_PASS)
 #endif
 LOOP_PASS("canon-freeze", CanonicalizeFreezeInLoopsPass())
+LOOP_PASS("dot-ddg", DDGDotPrinterPass())
 LOOP_PASS("invalidate", InvalidateAllAnalysesPass())
 LOOP_PASS("licm", LICMPass())
 LOOP_PASS("loop-idiom", LoopIdiomRecognizePass())
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -29,6 +29,7 @@
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/DDG.h"
+#include "llvm/Analysis/DDGPrinter.h"
 #include "llvm/Analysis/Delinearization.h"
 #include "llvm/Analysis/DemandedBits.h"
 #include "llvm/Analysis/DependenceAnalysis.h"
Index: llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
===
--- llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
+++ llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
@@ -35,7 +35,7 @@
   return true;
 }
 
-static bool isNodeHidden(const SUnit *Node) {
+static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
   return (Node->NumPreds > 10 || Node->NumSuccs > 10);
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -3836,7 +3836,7 @@
 return true;
   }
 
-  static bool isNodeHidden(const SUnit *Node) {
+  static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
 if (ViewMISchedCutoff == 0)
   return false;
 return (Node->Preds.size() > ViewMISchedCutoff
Index: llvm/lib/Analysis/DDGPrinter.cpp
===
--- /dev/null
+++ llvm/lib/Analysis/DDGPrinter.cpp
@@ -0,0 +1,150 @@
+//===- DDGPrinter.cpp - DOT printer for the data dependence graph --==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+//
+// This file defines the `-dot-ddg` analysis pass, which emits DDG in DOT format
+// in a file named `ddg..dot` for each loop  in a function.
+//===--===//
+
+#include "llvm/Analysis/DDGPrinter.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/GraphWriter.h"
+
+using namespace llvm;
+
+static cl::opt DotOnly("dot-ddg-only", cl::init(false), cl::Hidden,
+ cl::ZeroOrMore, cl::desc("simple ddg dot graph"));
+static cl::opt DDGDotFilenamePrefix(
+"dot-ddg-filename-prefix", cl::init("ddg"), cl::Hidden,
+cl::desc("The prefix used for the DDG dot file names."));
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly = false);
+
+//======//
+// Implementation of DDG DOT Printer for a loop
+//======//
+PreservedAnalyses DDGDotPrinterPass::run(Loop &L, LoopAnalysisManager &AM,
+ LoopStandardAnalysisResults &AR,
+ LPMUpdater &U) {
+  writeDDGToDotFile(*AM.getResult(L, AR), DotOnly);
+  return PreservedAnalyses::all();
+}
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly) {
+  std::string Filename =
+  Twine(DDGDotFilenamePrefix + "." + G.getName() + ".dot").str();
+  errs() << "Writing '" << Filename << "'...";
+
+  std::error_code EC;
+  raw_fd_ostream File(Filename, EC, s

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D90159#2453805 , @Meinersbur wrote:

> Can I help fixing the Windows build problem?

I think I have a fix (please see the updated patch), but don't have access to a 
windows machine to verify. Would you be able to try building with MSVC and let 
me know if it passes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour updated this revision to Diff 311897.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  llvm/include/llvm/Analysis/CFGPrinter.h
  llvm/include/llvm/Analysis/DDG.h
  llvm/include/llvm/Analysis/DDGPrinter.h
  llvm/include/llvm/Support/DOTGraphTraits.h
  llvm/include/llvm/Support/GraphWriter.h
  llvm/lib/Analysis/CFGPrinter.cpp
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/CallPrinter.cpp
  llvm/lib/Analysis/DDGPrinter.cpp
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def

Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -384,6 +384,7 @@
 #define LOOP_PASS(NAME, CREATE_PASS)
 #endif
 LOOP_PASS("canon-freeze", CanonicalizeFreezeInLoopsPass())
+LOOP_PASS("dot-ddg", DDGDotPrinterPass())
 LOOP_PASS("invalidate", InvalidateAllAnalysesPass())
 LOOP_PASS("licm", LICMPass())
 LOOP_PASS("loop-idiom", LoopIdiomRecognizePass())
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -29,6 +29,7 @@
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/DDG.h"
+#include "llvm/Analysis/DDGPrinter.h"
 #include "llvm/Analysis/Delinearization.h"
 #include "llvm/Analysis/DemandedBits.h"
 #include "llvm/Analysis/DependenceAnalysis.h"
Index: llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
===
--- llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
+++ llvm/lib/CodeGen/ScheduleDAGPrinter.cpp
@@ -35,7 +35,7 @@
   return true;
 }
 
-static bool isNodeHidden(const SUnit *Node) {
+static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
   return (Node->NumPreds > 10 || Node->NumSuccs > 10);
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -3836,7 +3836,7 @@
 return true;
   }
 
-  static bool isNodeHidden(const SUnit *Node) {
+  static bool isNodeHidden(const SUnit *Node, const ScheduleDAG *G) {
 if (ViewMISchedCutoff == 0)
   return false;
 return (Node->Preds.size() > ViewMISchedCutoff
Index: llvm/lib/Analysis/DDGPrinter.cpp
===
--- /dev/null
+++ llvm/lib/Analysis/DDGPrinter.cpp
@@ -0,0 +1,150 @@
+//===- DDGPrinter.cpp - DOT printer for the data dependence graph --==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+//
+// This file defines the `-dot-ddg` analysis pass, which emits DDG in DOT format
+// in a file named `ddg..dot` for each loop  in a function.
+//===--===//
+
+#include "llvm/Analysis/DDGPrinter.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/GraphWriter.h"
+
+using namespace llvm;
+
+static cl::opt DotOnly("dot-ddg-only", cl::init(false), cl::Hidden,
+ cl::ZeroOrMore, cl::desc("simple ddg dot graph"));
+static cl::opt DDGDotFilenamePrefix(
+"dot-ddg-filename-prefix", cl::init("ddg"), cl::Hidden,
+cl::desc("The prefix used for the DDG dot file names."));
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly = false);
+
+//======//
+// Implementation of DDG DOT Printer for a loop
+//======//
+PreservedAnalyses DDGDotPrinterPass::run(Loop &L, LoopAnalysisManager &AM,
+ LoopStandardAnalysisResults &AR,
+ LPMUpdater &U) {
+  writeDDGToDotFile(*AM.getResult(L, AR), DotOnly);
+  return PreservedAnalyses::all();
+}
+
+static void writeDDGToDotFile(DataDependenceGraph &G, bool DOnly) {
+  std::string Filename =
+  Twine(DDGDotFilenamePrefix + "." + G.getName() + ".dot").str();
+  errs() << "Writing '" << Filename << "'...";
+
+  std::error_code EC;
+  raw_fd_ostream File(Filename, EC, sys::fs::F_Text);
+
+  if (!EC)
+// We only provide the constant verson of the DOTGraphTrait specialization,
+// hence the conversion to const pointer
+WriteGraph(File, (co

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D90159#2454905 , @bmahjour wrote:

> In D90159#2453805 , @Meinersbur 
> wrote:
>
>> Can I help fixing the Windows build problem?
>
> I think I have a fix (please see the updated patch), but don't have access to 
> a windows machine to verify. Would you be able to try building with MSVC and 
> let me know if it passes?

I'll try to recommit and watch the windows buildbots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78938: Make LLVM build in C++20 mode

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40
   /// Static polymorphism: delegate implementation (via isEqualTo) to the
   /// derived class.
+  bool operator==(const DGEdge &E) const {

jfb wrote:
> That comment, so informative! 😐
It would be better to make these non-member functions as well, to be consistent 
with the `DGNode`.
```
  friend bool operator==(const EdgeType &Elhs, const EdgeType &Erhs) {
return Elhs.isEqualTo(Erhs);
  }
  friend bool operator!=(const EdgeType &Elhs, const EdgeType &Erhs) {
return !(Elhs == Erhs);
  }
```



Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40
   /// Static polymorphism: delegate implementation (via isEqualTo) to the
   /// derived class.
+  bool operator==(const DGEdge &E) const {

bmahjour wrote:
> jfb wrote:
> > That comment, so informative! 😐
> It would be better to make these non-member functions as well, to be 
> consistent with the `DGNode`.
> ```
>   friend bool operator==(const EdgeType &Elhs, const EdgeType &Erhs) {
> return Elhs.isEqualTo(Erhs);
>   }
>   friend bool operator!=(const EdgeType &Elhs, const EdgeType &Erhs) {
> return !(Elhs == Erhs);
>   }
> ```
> That comment, so informative! 😐

Yeah, it's not the best comment. It is trying to say that //the `isEqualTo` 
function gets selected, based on the type of the derived class in the CRTP, and 
that the selection is done at compile-time using type information, rather than 
at runtime using dynamic polymorphism//. Please feel free to update it to say 
that or offer any other suggestions you might have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/lib/Headers/altivec.h:17337
 static __inline__ vector long long __ATTRS_o_ai
 vec_vbpermq(vector unsigned char __a, vector unsigned char __b) {
   return __builtin_altivec_vbpermq(__a, __b);

lei wrote:
> bmahjour wrote:
> > This should be guarded under P8. It would also be good to add a 
> > `vec_vbpermd(vector unsigned long long ...)` counter part under 
> > `__POWER9_VECTOR__` for consistency.
> I think this is actually guarded under ` __POWER8_VECTOR__`.  See line 17237.
> As far as I can see, there is no LLVM intrinsic corresponding to vbpermd.
> I think this is actually guarded under  __POWER8_VECTOR__. See line 17237.

You are right. I missed that.

> As far as I can see, there is no LLVM intrinsic corresponding to vbpermd.

`__builtin_altivec_vbpermd` is being called on line 17356...not sure if it 
corresponds to an IR intrinsic or not, but regardless we have `vec_vbpermq` in 
this header but not `vec_vbpermd`. That seems inconsistent to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107899

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/lib/Headers/altivec.h:17349
+static __inline__ vector unsigned char __ATTRS_o_ai
+vec_bperm(vector unsigned char __a, vector unsigned char __b) {
+  return __builtin_altivec_vbpermq(__a, __b);

bmahjour wrote:
> `vbpermq` variants should be guarded under `__POWER8_VECTOR__`
never mind, looks like it already is guarded under P8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107899

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

As far as I can see, there is no good reason for the special treatment of 
vector bool/pixel going forward. Could we drop this special treatment, or at 
least change the default to use a scalar results across the board (consistent 
with XL's behaviour and clang's current behaviour for most cases).




Comment at: clang/include/clang/Driver/Options.td:3811
   MarshallingInfoFlag>;
+def vector_abi_compat : Joined<["-"], "vector-abi-compat=">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Determines whether vector compare returns a vector or a scalar. 
Options: default, gcc, xl.">,

I'm not sure the term "ABI" is really applicable. Maybe we should call it 
"vector-compare-compat="



Comment at: clang/test/CodeGen/vector-compat-pixel-bool-ternary.c:6
+// RUN:   -vector-abi-compat=gcc -triple powerpc-unknown-unknown -S -emit-llvm 
%s -o - 2>&1| FileCheck %s --check-prefix=ERROR
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+// RUN:   -vector-abi-compat=xl -triple powerpc-unknown-unknown -S -emit-llvm 
%s -o - | FileCheck %s

I only see the clang FE interface being tested. Does this have to be specified 
through `-Xclang -vector-abi-compat=...` or is there a clang driver option for 
it as well? I think we should have a clang driver option and have at least one 
test for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:248
+  enum class VectorCompareCompatKind {
+// Default clang behaviour.
+// All vector compares produce scalars except vector Pixel and vector bool.

How about adding `Default = Mixed` at the end and moving the comment with it?



Comment at: clang/include/clang/Basic/LangOptions.h:249
+// Default clang behaviour.
+// All vector compares produce scalars except vector Pixel and vector bool.
+// The types vector Pixel and vector bool return vector results.

Pixel -> pixel



Comment at: clang/include/clang/Basic/LangOptions.h:250
+// All vector compares produce scalars except vector Pixel and vector bool.
+// The types vector Pixel and vector bool return vector results.
+Mixed,

[nit] Pixel -> pixel



Comment at: clang/lib/Sema/SemaExpr.cpp:12221
+  // which behavior is prefered.
+  switch (getLangOpts().getVectorCompareCompat()) {
+  case LangOptions::VectorCompareCompatKind::Mixed:

Check for `getLangOpts().AltiVec` before the switch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

Sorry I didn't mention this in my earlier comment about the option name, but I 
think that all inconsistencies in handling vector bool/pixel types should be 
controlled by a single compatibility option. For example the current special 
handling of initialization (splat vs non-splat) for these types should also be 
option controlled. For that reason we should design and name the option in a 
more generic way. The same consideration should go into the wording of the 
deprecation warning.
Some suggestions:

rename `-vector-compare-compat` to `-altivec-compatibility` or 
`-altivec-compat` or  '-altivec-bool-pixel-compat` or 
`-vector-bool-pixel-compat`. (I prefer the last two)
reword the message to:
"Current handling of vector bool and vector pixel types in this context are 
deprecated. The default behaviour will soon change to that implied by the 
'-altivec-compat=xl' option"

Please also update the description section of this review.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7442
+  "pixel. Comparisons between vector bool/pixel types will soon return "
+  "a scalar value instead of a vector value by default">;
+

We need to add a new grouping (see `InGroup<...>`) for this, otherwise users 
won't be able to turn this warning off individually. There is also a clang LIT 
test that would fail if a warning message is added without a grouping 
specified. Please make sure check-all passes.



Comment at: clang/include/clang/Basic/LangOptions.def:130
+ENUM_LANGOPT(VectorCompareCompat, VectorCompareCompatKind, 2,
+ VectorCompareCompatKind::Mixed, "vector compare compatibility")
 LANGOPT(ConvergentFunctions, 1, 1, "Assume convergent functions")

`VectorCompareCompatKind::Mixed` -> `VectorCompareCompatKind::Default`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102094: [AIX][PowerPC] Remove error when specifying mabi=vec-default on AIX

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

LGTM...I'll approve this change unless there are any objections by EOD tomorrow.


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

https://reviews.llvm.org/D102094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/lib/Headers/altivec.h:17337
 static __inline__ vector long long __ATTRS_o_ai
 vec_vbpermq(vector unsigned char __a, vector unsigned char __b) {
   return __builtin_altivec_vbpermq(__a, __b);

This should be guarded under P8. It would also be good to add a 
`vec_vbpermd(vector unsigned long long ...)` counter part under 
`__POWER9_VECTOR__` for consistency.



Comment at: clang/lib/Headers/altivec.h:17349
+static __inline__ vector unsigned char __ATTRS_o_ai
+vec_bperm(vector unsigned char __a, vector unsigned char __b) {
+  return __builtin_altivec_vbpermq(__a, __b);

`vbpermq` variants should be guarded under `__POWER8_VECTOR__`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107899

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour accepted this revision.
bmahjour added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

@jansvoboda11 This change is causing the following LIT tests to fail on AIX:

  Clang :: ClangScanDeps/headerwithdirname.cpp
  Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp

The reason seems to be related to the fact that `-fno-integrated-as` is on by 
default on that platform. I get the same failure on Linux if I change the 
"compilation database" file to add `-fno-integrated-as` to the "command" line:

  > ./bin/clang-scan-deps -compilation-database ./headerwithdirname.cpp.tmp.cdb 
-j 1
  > Error while scanning dependencies for 
/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/headerwithdirname_input.cpp:
  error: unable to handle compilation, expected exactly one compiler job in ' 
"clang" "-cc1" "-triple" "powerpc64le-unknown-linux-gnu" "-S" ...;  
"/usr/bin/as" "-a64" "-mppc64" "-mlittle-endian" "-mpower8" "-I" 
"/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir"
 "-I" 
"/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/foodir"
 "-I" "Inputs" "-o" "headerwithdirname_input.o" 
"/tmp/headerwithdirname_input-2e0050.s"; '


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103461

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D103615#2847047 , @stefanp wrote:

> I'm sorry I missed the asserts requirement.
> I will recommit this patch after I add `REQUIRES: asserts`.

Instead of disabling the tests for non-assert builds, can we just remove the 
`entry:` checks at the beginning of each function? The rest of the IR checks 
should pass since they use a regexp so they should match for either named or 
unnamed instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D103615#2846245 , @dblaikie wrote:

> As mentioned in the reverting commit - these tests fail in non-asserts 
> builds, because they assume named IR instructions (like the named entry BB 
> label), which aren't provided on a non-asserts build (there's a flag to turn 
> them on - but that's probably not the right fix - making the test resilient 
> to non-asserts IR is probably the right fix).
>
> Leaving non-asserts builds broken for 12 hours (maybe I'm the first one to 
> come across/report it - but I'd expect there are some buildbots that would 
> fail, etc) is quite a while - best to be avoided when possible.

This is a bit off topic, but I'm just curious about clang's rationale for 
producing different IR depending on whether asserts are on/off? Seems strange 
and undesirable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

> (generally: disabling the test in non-asserts builds isn't the right path, 
> modifying the test so it doesn't depend on asserts IR naming is the right 
> path)

Agreed.

> Yes, probably removing the entry: check would be sufficient - give it a test 
> locally and see how it goes. (it does mean the "CHECK-NEXT" after that (for 
> the first instruction) would have to be a plain "CHECK" - so that the test 
> could pass both in the presence and absence of the entry label.

Right.

> Yeah, seems like a weird choice to me too (though has been around a long 
> time, so folks are pretty used to it) - might be worth bringing it up on 
> llvm-dev. I think we now have a flag to enable this functionality that works 
> even in non-asserts builds (maybe?) so maybe if we just change the default 
> for assert builds so it's always opt-in via a flag, then it's consistent 
> between asserts and non-asserts builds.

Do you happen to know what that option is? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D103615#2852430 , @krasimir wrote:

> I'm unfamiliar with the altivec API. What's a reasonable source code update 
> that preserves the current default behavior `-altivec-src-compat=mixed` under 
> `-altivec-src-compat=xl`, i.e., under `-altivec-src-compat=xl` how would we 
> compare vector bool or vector pixel to produce a vector?

For `vector bool` comparison one could use `vec_cmpeq` and `vec_compne` (see 
https://www.ibm.com/docs/en/xl-c-and-cpp-linux/16.1.1?topic=functions-vec-cmpeq).
 However, `vec_cmp*` interfaces do not currently support `vector pixel`. Do you 
have a concrete use case for comparing `vector pixel`s? If so I think we should 
consider adding corresponding overloads to `altivec.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105666: [PowerPC] [Altivec] Use signed comparison for vec_all_* and vec_any_* interfaces that compare vector bool types with vector signed types

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour created this revision.
bmahjour added reviewers: nemanjai, cebowleratibm, PowerPC, rzurob.
bmahjour added projects: PowerPC, LLVM.
Herald added subscribers: shchenz, kbarton.
bmahjour requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We are currently being inconsistent in using signed vs unsigned comparisons for 
vec_all_* and vec_any_* interfaces that use vector bool types. For example we 
use signed comparison for `vec_all_ge(vector signed char, vector bool char)` 
but unsigned comparison for when the arguments are swapped. GCC and XL use 
signed comparison instead. This patch makes clang consistent with XL and GCC.

Example:

  #include 
  #include 
  int main() {
vector signed char a = {0,0,0,0,0,0,0xA3,0,0,0x89,0,0,0xA4,0,0,0};
vector bool char b = {0,0,0,0,0,0,0xF3,0,0,0x61,0,0,0xAE,0,0,0};
printf(" a >= b : %d\n b >= a : %d\n", vec_all_ge(a, b), vec_all_ge(b, a));
return 0;
  }

currently produces

  a >= b : 0
  b >= a : 0

with this patch we get:

  a >= b : 0
  b >= a : 1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105666

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-altivec.c
  clang/test/CodeGen/builtins-ppc-vsx.c

Index: clang/test/CodeGen/builtins-ppc-vsx.c
===
--- clang/test/CodeGen/builtins-ppc-vsx.c
+++ clang/test/CodeGen/builtins-ppc-vsx.c
@@ -2724,8 +2724,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_ge(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_ge(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2753,8 +2753,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_gt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_gt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2782,8 +2782,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_le(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_le(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2811,8 +2811,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_lt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_lt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2840,8 +2840,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_ge(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_ge(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2869,8 +2869,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_gt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_gt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2898,8 +2898,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_le(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_le(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2927,8 +2927,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_lt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_lt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
Index: clang/test/CodeGen/builtins-ppc-altivec.c
===
--- clang/test/CodeGen/builtins-ppc-altivec.c
+++ clang/test/CodeGen/builtins-ppc-altivec.c
@@ -8097,8 +8097,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
 
   res_i = vec_all_ge(vbc, vsc);
-// CHECK: @llvm.ppc.altivec.vcmpgtub.p
-// CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
+// CHECK: @llvm.ppc.altivec.vcmpgtsb.p
+// CHECK-LE: @llvm.ppc.altivec.vcmpgtsb.p
 
   res_i = vec_all_ge(vbc, vuc);
 // CHECK: @llvm.ppc.altivec.vcmpgtub.p
@@ -8125,8 +8125,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtuh.p
 
   res_i = vec_all_ge(vbs, vs);
-// CHECK: @llvm.ppc.

[PATCH] D105666: [PowerPC] [Altivec] Use signed comparison for vec_all_* and vec_any_* interfaces that compare vector bool types with vector signed types

[Bardia Mahjour via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2071ce9d4559: [Altivec] Use signed comparison for vec_all_* 
and vec_any_* interfaces (authored by bmahjour).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105666

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-altivec.c
  clang/test/CodeGen/builtins-ppc-vsx.c

Index: clang/test/CodeGen/builtins-ppc-vsx.c
===
--- clang/test/CodeGen/builtins-ppc-vsx.c
+++ clang/test/CodeGen/builtins-ppc-vsx.c
@@ -2724,8 +2724,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_ge(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_ge(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2753,8 +2753,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_gt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_gt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2782,8 +2782,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_le(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_le(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2811,8 +2811,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_all_lt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_all_lt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2840,8 +2840,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_ge(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_ge(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2869,8 +2869,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_gt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_gt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2898,8 +2898,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_le(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_le(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
@@ -2927,8 +2927,8 @@
   // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
 
   res_i = vec_any_lt(vbll, vsll);
-  // CHECK: @llvm.ppc.altivec.vcmpgtud.p
-  // CHECK-LE: @llvm.ppc.altivec.vcmpgtud.p
+  // CHECK: @llvm.ppc.altivec.vcmpgtsd.p
+  // CHECK-LE: @llvm.ppc.altivec.vcmpgtsd.p
 
   res_i = vec_any_lt(vbll, vull);
   // CHECK: @llvm.ppc.altivec.vcmpgtud.p
Index: clang/test/CodeGen/builtins-ppc-altivec.c
===
--- clang/test/CodeGen/builtins-ppc-altivec.c
+++ clang/test/CodeGen/builtins-ppc-altivec.c
@@ -8097,8 +8097,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
 
   res_i = vec_all_ge(vbc, vsc);
-// CHECK: @llvm.ppc.altivec.vcmpgtub.p
-// CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
+// CHECK: @llvm.ppc.altivec.vcmpgtsb.p
+// CHECK-LE: @llvm.ppc.altivec.vcmpgtsb.p
 
   res_i = vec_all_ge(vbc, vuc);
 // CHECK: @llvm.ppc.altivec.vcmpgtub.p
@@ -8125,8 +8125,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtuh.p
 
   res_i = vec_all_ge(vbs, vs);
-// CHECK: @llvm.ppc.altivec.vcmpgtuh.p
-// CHECK-LE: @llvm.ppc.altivec.vcmpgtuh.p
+// CHECK: @llvm.ppc.altivec.vcmpgtsh.p
+// CHECK-LE: @llvm.ppc.altivec.vcmpgtsh.p
 
   res_i = vec_all_ge(vbs, vus);
 // CHECK: @llvm.ppc.altivec.vcmpgtuh.p
@@ -8153,8 +8153,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtuw.p
 
   res_i = vec_all_ge(vbi, vi);
-// CHECK: @llvm.ppc.altivec.vcmpgtuw.p
-// CHECK-LE: @llvm.ppc.altivec.vcmpgtuw.p
+// CHECK: @llvm.ppc.altivec.vcmpgtsw.p
+// CHECK-LE: @llvm.ppc.altivec.vcmpgtsw.p
 
   res_i = vec_all_ge(vbi, vui);
 // CHECK: @llvm.ppc.altivec.vcmpgtuw.p
@@ -8186,8 +8186,8 @@
 // CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
 
   res_i = vec_all_gt(vbc, vsc);
-// CHECK: @llvm.ppc.altivec.vcmpgtub.p
-// CHECK-LE: @llvm.ppc.altivec.vcmpgtub.p
+// CHECK: 

[PATCH] D114088: [PowerPC] Add BCD add/sub/cmp builtins

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

@nemanjai how come the changes in `altivec.h` and 
`clang/test/CodeGen/builtins-ppc-p8vector.c` have been removed in the latest 
diff and the commit above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114088

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: lld/test/ELF/lto/discard-value-names.ll:4
 
 ; RUN: ld.lld -shared -save-temps %t.o -o %t2.o
 ; RUN: llvm-dis < %t2.o.0.0.preopt.bc | FileCheck %s

I see `--plugin-opt=opaque-pointers` is being explicitly specified for some but 
not all tests. I suppose the reason you explicitly specify it is to make sure 
the tests pass even for builds where opaque pointers are disabled. If that's 
the case, why aren't you doing it consistently?



Comment at: llvm/test/Analysis/StackSafetyAnalysis/ipa.ll:86
 
-; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t.lto -stack-safety-print 
-stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN: llvm-lto2 run -opaque-pointers=0 %t.summ0.bc %t.summ1.bc -o %t.lto 
-stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
 ; RUN:  $(cat %t.res.txt) \

why does this test (and ipa-alias.ll above) need to run with opaque pointers 
off?



Comment at: llvm/test/LTO/Resolution/X86/alias-alias.ll:3
 ; RUN: llvm-as %p/Inputs/alias-alias-1.ll -o %t2.o
 ; RUN: llvm-lto2 run -o %t3.o %t1.o %t2.o -r %t2.o,a, -r %t2.o,d,px -r 
%t1.o,a,p -r %t1.o,c,p -r %t1.o,b -save-temps
 ; RUN: llvm-dis < %t3.o.0.0.preopt.bc -o - | FileCheck %s

Why do you pass `-lto-opaque-pointers` to llvm-lto2 in clang/tests but not here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:386
+  if (TM.getOptLevel() == CodeGenOpt::Aggressive){
+setOperationAction(ISD::FSIN , MVT::f64, Custom);
+setOperationAction(ISD::FCOS , MVT::f64, Custom);

what about tan, acos, and the others?



Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-afn-mass.ll:148
+
+attributes #1 = { "approx-func-fp-math"="true" }

All the calls have `afn`why do we need this attribute? 



Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-fast-mass.ll:148
+
+attributes #1 = { "no-infs-fp-math"="true" "no-nans-fp-math"="true" 
"no-signed-zeros-fp-math"="true" "approx-func-fp-math"="true" }

do we need this attribute? Can we remove it or have separate tests for 
functions with attributes?



Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-mass-aix.ll:1
+; RUN: llc -O3 -mtriple=powerpc-ibm-aix-xcoff < %s | FileCheck %s
+

We don't really need a separate aix file. Can we just add a run line with the 
aix triple to `llvm/test/CodeGen/PowerPC/lower-intrinsics-nofast-mass.ll`?



Comment at: llvm/test/CodeGen/PowerPC/lower-scalar-mass-fast.ll:796
+; Without nnan ninf afn nsz flags on the call instruction
+define float @acosf_f32_nofast(float %a) {
+; CHECK-LABEL: acosf_f32_nofast

shouldn't the tests starting from here move to a different file? This test file 
is called  ...mass-fast.ll so one would expect it only contains tests with 
fast-math flag on.



Comment at: 
llvm/test/CodeGen/PowerPC/pow-025-075-intrinsic-scalar-mass-fast.ll:246
+; CHECK-LNX-NEXT:lfs 2, .LCPI4_0@toc@l(3)
+; CHECK-LNX-NEXT:bl __xl_powf_finite
+; CHECK-LNX-NEXT:nop

How come pow -> sqrt conversion didn't happen here? 



Comment at: 
llvm/test/CodeGen/PowerPC/pow-025-075-nointrinsic-scalar-mass-fast.ll:22
+; CHECK-LNX-NEXT:lfs 2, .LCPI0_0@toc@l(3)
+; CHECK-LNX-NEXT:bl __xl_powf_finite
+; CHECK-LNX-NEXT:nop

so pow->sqrt translation never happens for non-intrinsic `pow`. Is that 
expected? If so, are we planning to recognize these patterns inside 
PPCGenScalarMASSEntries in the future and do the translation as part of that 
transform?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour accepted this revision.
bmahjour added a comment.
This revision is now accepted and ready to land.

Apart from some minor inline comments this revision addresses all my 
outstanding comments. LGTM.




Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:17747
+
+SDValue PPCTargetLowering::lowerLibCallType(const char *LibCallFloatName,
+const char *LibCallDoubleName,

[nit] a better name would be `lowerLibCallBasedOnType`



Comment at: 
llvm/test/CodeGen/PowerPC/pow-025-075-intrinsic-scalar-mass-fast.ll:246
+; CHECK-LNX-NEXT:lfs 2, .LCPI4_0@toc@l(3)
+; CHECK-LNX-NEXT:bl __xl_powf_finite
+; CHECK-LNX-NEXT:nop

masoud.ataei wrote:
> bmahjour wrote:
> > How come pow -> sqrt conversion didn't happen here? 
> Honestly, I am not sure why the conversion is not happening in this case. But 
> without this patch we will get `powf` call (the conversion is not happening 
> again). So this is a separate issue that someone needs to look at independent 
> of this patch.
Could you please make a note of this as a todo comment in each test that is 
affected?



Comment at: 
llvm/test/CodeGen/PowerPC/pow-025-075-nointrinsic-scalar-mass-fast.ll:22
+; CHECK-LNX-NEXT:lfs 2, .LCPI0_0@toc@l(3)
+; CHECK-LNX-NEXT:bl __xl_powf_finite
+; CHECK-LNX-NEXT:nop

masoud.ataei wrote:
> bmahjour wrote:
> > so pow->sqrt translation never happens for non-intrinsic `pow`. Is that 
> > expected? If so, are we planning to recognize these patterns inside 
> > PPCGenScalarMASSEntries in the future and do the translation as part of 
> > that transform?
> Correct, pow->sqrt translation is not happening for none intrinsic cases. It 
> is the case independent of this patch. I guess the reason is DAGCombiner only 
> apply this optimization on llvm intrinsics. This is an issue that either we 
> need to handle it in DAGCombiner (same as intrinsic one) or in MASS pass. I 
> feel DAGCombiner is a better option and I think this is also a separate 
> issue. 
Ok, I understand now. We'll have to come back to this later at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101130: [PowerPC] Provide XL-compatible builtins in altivec.h

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

LGTM




Comment at: clang/test/CodeGen/builtins-ppc-xlcompat.c:6
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+  // RUN:   -triple powerpc64le-unknown-unknown -emit-llvm %s -o - \
+// RUN:   -D__XL_COMPAT_ALTIVEC__ | FileCheck %s

[nit] remove extra space


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101130

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101209: [PowerPC] Provide fastmath sqrt and div functions in altivec.h

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour requested changes to this revision.
bmahjour added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15130
+  Value *Y = EmitScalarExpr(E->getArg(1));
+  auto Ret = Builder.CreateFDiv(X, Y, "recipdiv");
+  Builder.setFastMathFlags(FMF);

I wonder if we can do better than "fdiv fast"... does the current lowering of 
"fdiv fast" employ an estimation algorithm via iterative refinement on POWER?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15134
+}
+llvm::Function *F = CGM.getIntrinsic(Intrinsic::sqrt, ResultType);
+auto Ret = Builder.CreateCall(F, X);

This doesn't implement a reciprocal square root, it just performs a square 
root! At the very least we need a divide instruction following the call to the 
intrinsic, but I'm not sure if that'll result in the most optimal codegen at 
the end. Perhaps we need a new builtin?



Comment at: clang/test/CodeGen/builtins-ppc-vsx.c:2297
+  // CHECK-LABEL: test_rsqrtd
+  // CHECK: call fast <2 x double> @llvm.sqrt.v2f64
+  // CHECK-LE-LABEL: test_rsqrtd

See my comment above about the missing reciprocal operation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101209

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101209: [PowerPC] Provide fastmath sqrt and div functions in altivec.h

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour accepted this revision.
bmahjour added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101209

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: 
llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:273-276
-; VF-TWO-CHECK-DAG: [[LOOPID_MV]] = distinct !{[[LOOPID_MV]], 
[[LOOPID_DISABLE_VECT:!.*]]}
-; VF-TWO-CHECK-DAG: [[LOOPID_EV]] = distinct !{[[LOOPID_EV]], 
[[LOOPID_DISABLE_UNROLL:!.*]], [[LOOPID_DISABLE_VECT:!.*]]}
-; VF-TWO-CHECK-DAG: [[LOOPID_DISABLE_VECT]] = 
[[DISABLE_VECT_STR:!{!"llvm.loop.isvectorized".*}.*]]
-; VF-TWO-CHECK-DAG: [[LOOPID_DISABLE_UNROLL]] = 
[[DISABLE_UNROLL_STR:!{!"llvm.loop.unroll.runtime.disable"}.*]]

these lines should not have been removed!



Comment at: 
llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:556-559
-; VF-FOUR-CHECK-DAG: [[LOOPID_MV_CM]] = distinct !{[[LOOPID_MV_CM]], 
[[LOOPID_DISABLE_VECT_CM:!.*]]}
-; VF-FOUR-CHECK-DAG: [[LOOPID_EV_CM]] = distinct !{[[LOOPID_EV_CM]], 
[[LOOPID_DISABLE_UNROLL_CM:!.*]], [[LOOPID_DISABLE_VECT_CM:!.*]]}
-; VF-FOUR-CHECK-DAG: [[LOOPID_DISABLE_VECT_CM]] = 
[[DISABLE_VECT_STR_CM:!{!"llvm.loop.isvectorized".*}.*]]
-; VF-FOUR-CHECK-DAG: [[LOOPID_DISABLE_UNROLL_CM]] = 
[[DISABLE_UNROLL_STR_CM:!{!"llvm.loop.unroll.runtime.disable"}.*]]

these lines should not have been removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: 
llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:273-276
-; VF-TWO-CHECK-DAG: [[LOOPID_MV]] = distinct !{[[LOOPID_MV]], 
[[LOOPID_DISABLE_VECT:!.*]]}
-; VF-TWO-CHECK-DAG: [[LOOPID_EV]] = distinct !{[[LOOPID_EV]], 
[[LOOPID_DISABLE_UNROLL:!.*]], [[LOOPID_DISABLE_VECT:!.*]]}
-; VF-TWO-CHECK-DAG: [[LOOPID_DISABLE_VECT]] = 
[[DISABLE_VECT_STR:!{!"llvm.loop.isvectorized".*}.*]]
-; VF-TWO-CHECK-DAG: [[LOOPID_DISABLE_UNROLL]] = 
[[DISABLE_UNROLL_STR:!{!"llvm.loop.unroll.runtime.disable"}.*]]

efriedma wrote:
> bmahjour wrote:
> > these lines should not have been removed!
> It looks like someone manually hacked on the CHECK lines in a test marked "; 
> NOTE: Assertions have been autogenerated by utils/update_test_checks.py".  
> This is generally a bad idea... it makes it easy to miss lines like this.  If 
> CHECKs are manually written or modified, the NOTE should be removed as a 
> warning to anyone modifying the test in the future.
> 
> Not sure what the best solution is here... at minimum, I guess we can just 
> drop the "autogenerated" note?
Yes, I should have removed that comment (lesson learned), although that 
wouldn't necessarily have prevented the oversight.

We cannot put those lines back until the regression I reported in 
https://reviews.llvm.org/D116479 is fixed. I can remove the autogen note at 
that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D90159#2466599 , @MaskRay wrote:

> Should this have some tests? Even if guarded by `REQUIRES:` if some feature 
> is needed.

Test coverage for the DDG functionality has been added under LIT and unittests. 
I've opened https://reviews.llvm.org/D93949 to add a test and verify the 
formatting of the dot file produced when using `-dot-ddg`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D101759#2971567 , @efriedma wrote:

> errno handling for math library functions is a mess.  Currently, we don't 
> model it properly; we just mark the calls "readnone" and hope for the best.  
> If you don't want to fix that, just check for readnone for now.

I think using `readnone` would work fine. It seems that clang marks math 
functions with that attribute when `-fno-math-errno` is in effect. To get the 
non-finite MASS lowerings at -O3 one would have to compile with both 
`-fapprox-func` and `-fno-math-errno`, which seems reasonable to me.

> I don't think we want to be querying function attributes or options here; afn 
> plus enabling MASS should be enough.  The function attributes are the old 
> mechanism; we just haven't completely migrated some parts of SelectionDAG yet.

I agree. I think the problem is that this patch is trying to decide on a global 
lowering strategy for `llvm.*` math intrinsics in 
`llvm/lib/Target/PowerPC/PPCISelLowering.cpp` but such global decision making 
does not go well with finer granularity of fast-math flags. My understanding is 
that the reason we need to handle //intrinsic// math functions later is because 
of strength-reduction transformations like `pow(x,0.5) --> sqrt(x)` that 
currently operate on intrinsic calls only. If we could apply those operations 
on things like `__xl_pow_finite` and produce calls to `__xl_sqrt_finite` then 
we wouldn't have this problem. Another possibility might be to have two 
versions of `PPCGenScalarMASSEntries` one that handles non-intrinsics and runs 
earlier, and another one that handles intrinsics after transformations likes 
`pow(x,0.5) --> sqrt(x)` are done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106191: [clang] Option control afn flag

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1732-1733
   NegFlag>;
-def fapprox_func : Flag<["-"], "fapprox-func">, Group, 
Flags<[CC1Option, NoDriverOption]>,
-  MarshallingInfoFlag>, 
ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>;
 defm finite_math_only : BoolFOption<"finite-math-only",

So this option already exists and seems to behave the way we want it to. Does 
anyone know why it was made `NoDriverOption`?

```
> cat fp.c
#include 
void foo(float *f1, float *f2)
{
  *f1 = sin(*f2) + *f1;
}
> clang -c fp.c -S -emit-llvm -mllvm -disable-llvm-optzns -O3 -Xclang 
> -fapprox-func && grep afn fp.ll
  %call = call afn double @sin(double %conv) #2
  %add = fadd afn double %call, %conv1
```

Could we just expose it as a supported option and call it done. ie make it more 
like `fhonor_nans` below but without introducing a new function attribute:

```def fapprox_func : Flag<["-"], "fapprox-func">, Group;```

so that instead of having `-Xclang -fapprox-func ` in the command above we 
could just have `-fapprox-func `?



Comment at: clang/test/CodeGen/afn-flag-test.c:10
+  // CHECK-AFN:  %{{.*}} = call afn double @{{.*}}exp{{.*}}(double %{{.*}})
+  // CHECK-AFN:  attributes #0 ={{.*}} "approx-func-fp-math"="true" {{.*}}
+

can we avoid these attributes?



Comment at: clang/test/CodeGen/afn-flag-test.c:13
+  // CHECK-NO-AFN:   %{{.*}} = call double @{{.*}}exp{{.*}}(double %{{.*}})
+  // CHECK-NO-AFN-NOT:  attributes #0 ={{.*}} "approx-func-fp-math"="true" 
{{.*}}
+}

avoid attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits