[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-13 Thread Alexey Bataev via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284110: Fix for PR30639: CGDebugInfo Null dereference with 
OpenMP array (authored by ABataev).

Changed prior to commit:
  https://reviews.llvm.org/D25373?vs=74386=74485#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25373

Files:
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/test/OpenMP/debug-info-openmp-array.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
  cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
  cfe/trunk/test/OpenMP/target_map_codegen.cpp

Index: cfe/trunk/test/OpenMP/target_map_codegen.cpp
===
--- cfe/trunk/test/OpenMP/target_map_codegen.cpp
+++ cfe/trunk/test/OpenMP/target_map_codegen.cpp
@@ -675,7 +675,7 @@
   }
 }
 
-// CK13: define internal void [[KERNEL]](i[[sz]] [[VLA0:%.+]], i[[sz]] [[VLA1:%.+]], double* {{.+}}[[ARG:%.+]])
+// CK13: define internal void [[KERNEL]](i[[sz]] [[VLA0:%.+]], i[[sz]] [[VLA1:%.+]], double* {{.*}}[[ARG:%.+]])
 // CK13: [[ADDR0:%.+]] = alloca i[[sz]],
 // CK13: [[ADDR1:%.+]] = alloca i[[sz]],
 // CK13: [[ADDR2:%.+]] = alloca double*,
Index: cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
===
--- cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
+++ cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
@@ -301,7 +301,7 @@
 // CHECK: fadd float 5.55e+02, %
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
 
 // Reduction list for runtime.
 // CHECK: [[RED_LIST:%.+]] = alloca [4 x i8*],
@@ -500,7 +500,7 @@
 
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
 
 // CHECK: [[ARRS_PRIV:%.+]] = alloca [10 x [4 x [[S_FLOAT_TY,
 
Index: cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
===
--- cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
+++ cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
@@ -492,7 +492,7 @@
 // CHECK: store float [[UP]], float* [[T_VAR1_LHS]],
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
 
 // Reduction list for runtime.
 // CHECK: [[RED_LIST:%.+]] = alloca [4 x i8*],
@@ -696,7 +696,7 @@
 
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
 
 // CHECK: [[ARRS_PRIV:%.+]] = alloca [10 x [4 x [[S_FLOAT_TY,
 
Index: cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
===
--- cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
+++ cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
@@ -222,7 +222,7 @@
   
   // make sure that firstprivate variables are generated in all cases and that we use those instances for operations inside the
   // target region
-  // TCHECK:  define void @__omp_offloading_{{.+}}(i{{[0-9]+}} [[A2_IN:%.+]], [10 x float]* {{.+}} [[B_IN:%.+]], i{{[0-9]+}} [[BN_SZ:%.+]], float* {{.+}} [[BN_IN:%.+]], [5 x [10 x double]]* {{.+}} [[C_IN:%.+]], i{{[0-9]+}} [[CN_SZ1:%.+]], i{{[0-9]+}} [[CN_SZ2:%.+]], double* {{.+}} [[CN_IN:%.+]], [[TT]]* {{.+}} [[D_IN:%.+]])
+  // TCHECK:  define

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-12 Thread Alexey Bataev via cfe-commits 
ABataev added a comment.

I will commit it for you, Erich, no problems


https://reviews.llvm.org/D25373



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

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-12 Thread Alexey Bataev via cfe-commits 
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG with a nit




Comment at: lib/CodeGen/CGStmtOpenMP.cpp:312
+  assert(ArgLVal.getType()->isPointerType());
+  ArgAddr = this->EmitLoadOfPointer(
+  ArgAddr, ArgLVal.getType()->castAs());

Remove `this->`


https://reviews.llvm.org/D25373



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

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-11 Thread Erich Keane via cfe-commits 
erichkeane updated the summary for this revision.
erichkeane removed a reviewer: gbenyei.
erichkeane removed rL LLVM as the repository for this revision.
erichkeane updated this revision to Diff 74310.
erichkeane added a comment.

Emailed with Alexey who identified the problem in the OpenMP implementation 
that was resulting in incomplete array types being emitted, which broke an 
assumption in the new CGDebugInfo code.


https://reviews.llvm.org/D25373

Files:
  lib/CodeGen/CGStmtOpenMP.cpp
  test/CodeGenCXX/debug-info-openmp-array.cpp
  test/OpenMP/for_reduction_codegen.cpp
  test/OpenMP/for_reduction_codegen_UDR.cpp
  test/OpenMP/target_firstprivate_codegen.cpp
  test/OpenMP/target_map_codegen.cpp

Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -232,8 +232,15 @@
   assert(I->capturesVariableArrayType());
   II = ().Idents.get("vla");
 }
-if (ArgType->isVariablyModifiedType())
-  ArgType = getContext().getVariableArrayDecayedType(ArgType);
+if (ArgType->isVariablyModifiedType()) {
+  bool IsReference = ArgType->isLValueReferenceType();
+  ArgType =
+  getContext().getCanonicalParamType(ArgType.getNonReferenceType());
+  if (IsReference && !ArgType->isPointerType()) {
+ArgType = getContext().getLValueReferenceType(
+ArgType, /*SpelledAsLValue=*/false);
+  }
+}
 Args.push_back(ImplicitParamDecl::Create(getContext(), nullptr,
  FD->getLocation(), II, ArgType));
 ++I;
@@ -297,8 +304,14 @@
   QualType VarTy = Var->getType();
   Address ArgAddr = ArgLVal.getAddress();
   if (!VarTy->isReferenceType()) {
-ArgAddr = EmitLoadOfReference(
-ArgAddr, ArgLVal.getType()->castAs());
+if (ArgLVal.getType()->isLValueReferenceType()) {
+  ArgAddr = EmitLoadOfReference(
+  ArgAddr, ArgLVal.getType()->castAs());
+} else {
+  assert(ArgLVal.getType()->isPointerType());
+  ArgAddr = this->EmitLoadOfPointer(
+  ArgAddr, ArgLVal.getType()->castAs());
+}
   }
   setAddrOfLocalVar(
   Var, Address(ArgAddr.getPointer(), getContext().getDeclAlign(Var)));
Index: test/OpenMP/for_reduction_codegen_UDR.cpp
===
--- test/OpenMP/for_reduction_codegen_UDR.cpp
+++ test/OpenMP/for_reduction_codegen_UDR.cpp
@@ -301,7 +301,7 @@
 // CHECK: fadd float 5.55e+02, %
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
 
 // Reduction list for runtime.
 // CHECK: [[RED_LIST:%.+]] = alloca [4 x i8*],
@@ -500,7 +500,7 @@
 
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(480) %{{.+}})
 
 // CHECK: [[ARRS_PRIV:%.+]] = alloca [10 x [4 x [[S_FLOAT_TY,
 
Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -675,7 +675,7 @@
   }
 }
 
-// CK13: define internal void [[KERNEL]](i[[sz]] [[VLA0:%.+]], i[[sz]] [[VLA1:%.+]], double* {{.+}}[[ARG:%.+]])
+// CK13: define internal void [[KERNEL]](i[[sz]] [[VLA0:%.+]], i[[sz]] [[VLA1:%.+]], double* {{.*}}[[ARG:%.+]])
 // CK13: [[ADDR0:%.+]] = alloca i[[sz]],
 // CK13: [[ADDR1:%.+]] = alloca i[[sz]],
 // CK13: [[ADDR2:%.+]] = alloca double*,
Index: test/OpenMP/for_reduction_codegen.cpp
===
--- test/OpenMP/for_reduction_codegen.cpp
+++ test/OpenMP/for_reduction_codegen.cpp
@@ -492,7 +492,7 @@
 // CHECK: store float [[UP]], float* [[T_VAR1_LHS]],
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* nonnull %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}*

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Keane, Erich via cfe-commits 
I don't see any .ll files for the tests.  I see the directory 
build/tools/clang/test/OpenMP/Output has a bunch of .script and .tmp(binary 
files), but no obvious .ll files.


-Original Message-
From: Alexey Bataev [mailto:a.bat...@hotmail.com] 
Sent: Monday, October 10, 2016 1:22 PM
To: Keane, Erich <erich.ke...@intel.com>
Cc: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
guy.ben...@intel.com
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

I mean, could you send new  .ll files, which you get after these changes?

Best regards,
Alexey Bataev

> 10 окт. 2016 г., в 22:26, Keane, Erich <erich.ke...@intel.com> написал(а):
> 
> Sure, attached.  So far, there are a few changes that seem curious and 
> concerning. A few cases of loads going missing, and a few more of things like 
> 'nonnull' going missing.
> 
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 12:18 PM
> To: Keane, Erich <erich.ke...@intel.com>; 
> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
> cfe-commits@lists.llvm.org; dblai...@gmail.com; 
> david.majne...@gmail.com; guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
> dereference with OpenMP array access
> 
> Could you send the new results for these tests?
> 
> Best regards,
> Alexey Bataev
> 
>> On 10/10/2016 09:22 PM, Keane, Erich wrote:
>> This causes a massive number of openmp test failures (obviously), and I'm 
>> not terribly comfortable with how OpenMP works.  I can update the tests (and 
>> will), but would like it if you could be extra diligent in confirming the 
>> correct behavior for me.
>> 
>> Failing Tests (6):
>> Clang :: OpenMP/for_reduction_codegen.cpp
>> Clang :: OpenMP/for_reduction_codegen_UDR.cpp
>> Clang :: OpenMP/nvptx_target_codegen.cpp
>> Clang :: OpenMP/target_codegen.cpp
>> Clang :: OpenMP/target_firstprivate_codegen.cpp
>> Clang :: OpenMP/target_map_codegen.cpp -Original Message-
>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>> Sent: Monday, October 10, 2016 10:55 AM
>> To: Keane, Erich <erich.ke...@intel.com>;
>> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org;
>> cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>> david.majne...@gmail.com; guy.ben...@intel.com
>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
>> dereference with OpenMP array access
>> 
>> Add a check to line 299:
>> 
>> if (!VarTy->isReferenceType() &&
>> !(FD->getType()->isVariablyModifiedType()  &&
>> ArgLVal.getType()->isPointerType()))
>> 
>> Best regards,
>> Alexey Bataev
>> 
>>> On 10/10/2016 08:19 PM, Keane, Erich wrote:
>>> That does turn ArgType into a PointerType int*.  However, line 301 
>>> (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference 
>>> type a bit later, resulting in a SIGABRT there.  Do I need to re-add the 
>>> reference there?  Is removing the array type REALLY what we wish to do?
>>> 
>>> -----Original Message-----
>>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>>> Sent: Monday, October 10, 2016 10:09 AM
>>> To: Keane, Erich <erich.ke...@intel.com>;
>>> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org;
>>> cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>>> david.majne...@gmail.com; guy.ben...@intel.com
>>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
>>> dereference with OpenMP array access
>>> 
>>> Aaaahhh,
>>> 
>>> Now I see. We have a parameter with the type that is a reference to VLA.
>>> I think in this case we should rework this code to something like this:
>>> 
>>> if (ArgType->isVariablyModifiedType())
>>> ArgType =
>>> getContext().getCanonicalParamType(ArgType.getNonReferenceType());
>>> 
>>> 
>>> Try this solution.
>>> 
>>> Best regards,
>>> Alexey Bataev
>>> 
>>>> On 10/10/2016 07:51 PM, Keane, Erich wrote:
>>>> I did.  I changed line 236 to "ArgType = 
>>>> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
>>>> suggesting), and the output was identical when doing dump on ArgType:
>>>> 
>>>> This:
>>>> 
>>>> LValueReferenceType 0xa451620 'int (&)[*]' var

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
I mean, could you send new  .ll files, which you get after these changes?

Best regards,
Alexey Bataev

> 10 окт. 2016 г., в 22:26, Keane, Erich <erich.ke...@intel.com> написал(а):
> 
> Sure, attached.  So far, there are a few changes that seem curious and 
> concerning. A few cases of loads going missing, and a few more of things like 
> 'nonnull' going missing.
> 
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com] 
> Sent: Monday, October 10, 2016 12:18 PM
> To: Keane, Erich <erich.ke...@intel.com>; 
> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
> cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
> guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
> 
> Could you send the new results for these tests?
> 
> Best regards,
> Alexey Bataev
> 
>> On 10/10/2016 09:22 PM, Keane, Erich wrote:
>> This causes a massive number of openmp test failures (obviously), and I'm 
>> not terribly comfortable with how OpenMP works.  I can update the tests (and 
>> will), but would like it if you could be extra diligent in confirming the 
>> correct behavior for me.
>> 
>> Failing Tests (6):
>> Clang :: OpenMP/for_reduction_codegen.cpp
>> Clang :: OpenMP/for_reduction_codegen_UDR.cpp
>> Clang :: OpenMP/nvptx_target_codegen.cpp
>> Clang :: OpenMP/target_codegen.cpp
>> Clang :: OpenMP/target_firstprivate_codegen.cpp
>> Clang :: OpenMP/target_map_codegen.cpp -Original Message-
>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>> Sent: Monday, October 10, 2016 10:55 AM
>> To: Keane, Erich <erich.ke...@intel.com>; 
>> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
>> cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>> david.majne...@gmail.com; guy.ben...@intel.com
>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
>> dereference with OpenMP array access
>> 
>> Add a check to line 299:
>> 
>> if (!VarTy->isReferenceType() &&
>> !(FD->getType()->isVariablyModifiedType()  &&
>> ArgLVal.getType()->isPointerType()))
>> 
>> Best regards,
>> Alexey Bataev
>> 
>>> On 10/10/2016 08:19 PM, Keane, Erich wrote:
>>> That does turn ArgType into a PointerType int*.  However, line 301 
>>> (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference 
>>> type a bit later, resulting in a SIGABRT there.  Do I need to re-add the 
>>> reference there?  Is removing the array type REALLY what we wish to do?
>>> 
>>> -----Original Message-----
>>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>>> Sent: Monday, October 10, 2016 10:09 AM
>>> To: Keane, Erich <erich.ke...@intel.com>; 
>>> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
>>> cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>>> david.majne...@gmail.com; guy.ben...@intel.com
>>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
>>> dereference with OpenMP array access
>>> 
>>> Aaaahhh,
>>> 
>>> Now I see. We have a parameter with the type that is a reference to VLA.
>>> I think in this case we should rework this code to something like this:
>>> 
>>> if (ArgType->isVariablyModifiedType())
>>> ArgType =
>>> getContext().getCanonicalParamType(ArgType.getNonReferenceType());
>>> 
>>> 
>>> Try this solution.
>>> 
>>> Best regards,
>>> Alexey Bataev
>>> 
>>>> On 10/10/2016 07:51 PM, Keane, Erich wrote:
>>>> I did.  I changed line 236 to "ArgType = 
>>>> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
>>>> suggesting), and the output was identical when doing dump on ArgType:
>>>> 
>>>> This:
>>>> 
>>>> LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>>>>     `-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>>>>   |-BuiltinType 0xa3f4090 'int'
>>>>   `-<<>>
>>>> Vs:
>>>> LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>>>> `-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>>>>   |-BuiltinType 0xa3f4090 'int'
>>>>   `-<<>>
>>>> 
>>>> -Original Message-
>>>> From: Alexey Bataev [mailto:a.bat...@hotmai

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
Could you send the new results for these tests?

Best regards,
Alexey Bataev

On 10/10/2016 09:22 PM, Keane, Erich wrote:
> This causes a massive number of openmp test failures (obviously), and I'm not 
> terribly comfortable with how OpenMP works.  I can update the tests (and 
> will), but would like it if you could be extra diligent in confirming the 
> correct behavior for me.
>
> Failing Tests (6):
>  Clang :: OpenMP/for_reduction_codegen.cpp
>  Clang :: OpenMP/for_reduction_codegen_UDR.cpp
>  Clang :: OpenMP/nvptx_target_codegen.cpp
>  Clang :: OpenMP/target_codegen.cpp
>  Clang :: OpenMP/target_firstprivate_codegen.cpp
>  Clang :: OpenMP/target_map_codegen.cpp
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 10:55 AM
> To: Keane, Erich <erich.ke...@intel.com>; 
> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
> cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
> guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
>
> Add a check to line 299:
>
> if (!VarTy->isReferenceType() &&
> !(FD->getType()->isVariablyModifiedType()  &&
> ArgLVal.getType()->isPointerType()))
>
> Best regards,
> Alexey Bataev
>
> On 10/10/2016 08:19 PM, Keane, Erich wrote:
>> That does turn ArgType into a PointerType int*.  However, line 301 
>> (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference type 
>> a bit later, resulting in a SIGABRT there.  Do I need to re-add the 
>> reference there?  Is removing the array type REALLY what we wish to do?
>>
>> -Original Message-
>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>> Sent: Monday, October 10, 2016 10:09 AM
>> To: Keane, Erich <erich.ke...@intel.com>; 
>> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
>> cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
>> guy.ben...@intel.com
>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
>> with OpenMP array access
>>
>> Aaaahhh,
>>
>> Now I see. We have a parameter with the type that is a reference to VLA.
>> I think in this case we should rework this code to something like this:
>>
>> if (ArgType->isVariablyModifiedType())
>>  ArgType =
>> getContext().getCanonicalParamType(ArgType.getNonReferenceType());
>>
>>
>> Try this solution.
>>
>> Best regards,
>> Alexey Bataev
>>
>> On 10/10/2016 07:51 PM, Keane, Erich wrote:
>>> I did.  I changed line 236 to "ArgType = 
>>> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
>>> suggesting), and the output was identical when doing dump on ArgType:
>>>
>>> This:
>>>
>>>  LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>>>  `-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>>>|-BuiltinType 0xa3f4090 'int'
>>>`-<<>>
>>> Vs:
>>>  LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>>>  `-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>>>    |-BuiltinType 0xa3f4090 'int'
>>>`-<<>>
>>>
>>> -Original Message-
>>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>>> Sent: Monday, October 10, 2016 9:47 AM
>>> To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
>>> <erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>>> david.majne...@gmail.com; guy.ben...@intel.com
>>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null 
>>> dereference with OpenMP array access
>>>
>>> Hmm,
>>>
>>> I thought it must return a pointer to the element type rather than [*] kind 
>>> type. Did you checked it?
>>>
>>> Best regards,
>>> Alexey Bataev
>>>
>>> On 10/10/2016 07:09 PM, Erich Keane wrote:
>>>> erichkeane added a comment.
>>>>
>>>> Andrey-
>>>> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
>>>> the same type in this case (at least in the reproduction).  I could 
>>>> definitely change the call, though the changes to CGDebugInfo.cpp are 
>>>> apparently also necessary even in that case.
>>>>
>>>> Is there additional logic that should happen in CGStmtOpenMP that I'm 
>>>> missing?
>>>>
>>>>
>>>> Repository:
>>>>   rL LLVM
>>>>
>>>> https://reviews.llvm.org/D25373
>>>>
>>>>
>>>>

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

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Keane, Erich via cfe-commits 
This causes a massive number of openmp test failures (obviously), and I'm not 
terribly comfortable with how OpenMP works.  I can update the tests (and will), 
but would like it if you could be extra diligent in confirming the correct 
behavior for me.

Failing Tests (6):
Clang :: OpenMP/for_reduction_codegen.cpp
Clang :: OpenMP/for_reduction_codegen_UDR.cpp
Clang :: OpenMP/nvptx_target_codegen.cpp
Clang :: OpenMP/target_codegen.cpp
Clang :: OpenMP/target_firstprivate_codegen.cpp
Clang :: OpenMP/target_map_codegen.cpp
-Original Message-
From: Alexey Bataev [mailto:a.bat...@hotmail.com] 
Sent: Monday, October 10, 2016 10:55 AM
To: Keane, Erich <erich.ke...@intel.com>; 
reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
guy.ben...@intel.com
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Add a check to line 299:

if (!VarTy->isReferenceType() && 
!(FD->getType()->isVariablyModifiedType()  && 
ArgLVal.getType()->isPointerType()))

Best regards,
Alexey Bataev

On 10/10/2016 08:19 PM, Keane, Erich wrote:
> That does turn ArgType into a PointerType int*.  However, line 301 
> (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference type 
> a bit later, resulting in a SIGABRT there.  Do I need to re-add the reference 
> there?  Is removing the array type REALLY what we wish to do?
>
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 10:09 AM
> To: Keane, Erich <erich.ke...@intel.com>; 
> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
> cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
> guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
>
> Aaaahhh,
>
> Now I see. We have a parameter with the type that is a reference to VLA.
> I think in this case we should rework this code to something like this:
>
> if (ArgType->isVariablyModifiedType())
> ArgType =
> getContext().getCanonicalParamType(ArgType.getNonReferenceType());
>
>
> Try this solution.
>
> Best regards,
> Alexey Bataev
>
> On 10/10/2016 07:51 PM, Keane, Erich wrote:
>> I did.  I changed line 236 to "ArgType = 
>> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
>> suggesting), and the output was identical when doing dump on ArgType:
>>
>> This:
>>
>> LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>> `-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>>   |-BuiltinType 0xa3f4090 'int'
>>   `-<<>>
>> Vs:
>> LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>> `-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>>   |-BuiltinType 0xa3f4090 'int'
>>   `-<<>>
>>
>> -Original Message-
>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>> Sent: Monday, October 10, 2016 9:47 AM
>> To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
>> <erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>> david.majne...@gmail.com; guy.ben...@intel.com
>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
>> with OpenMP array access
>>
>> Hmm,
>>
>> I thought it must return a pointer to the element type rather than [*] kind 
>> type. Did you checked it?
>>
>> Best regards,
>> Alexey Bataev
>>
>> On 10/10/2016 07:09 PM, Erich Keane wrote:
>>> erichkeane added a comment.
>>>
>>> Andrey-
>>> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
>>> the same type in this case (at least in the reproduction).  I could 
>>> definitely change the call, though the changes to CGDebugInfo.cpp are 
>>> apparently also necessary even in that case.
>>>
>>> Is there additional logic that should happen in CGStmtOpenMP that I'm 
>>> missing?
>>>
>>>
>>> Repository:
>>>  rL LLVM
>>>
>>> https://reviews.llvm.org/D25373
>>>
>>>
>>>

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

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
Add a check to line 299:

if (!VarTy->isReferenceType() && 
!(FD->getType()->isVariablyModifiedType()  && 
ArgLVal.getType()->isPointerType()))

Best regards,
Alexey Bataev

On 10/10/2016 08:19 PM, Keane, Erich wrote:
> That does turn ArgType into a PointerType int*.  However, line 301 
> (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference type 
> a bit later, resulting in a SIGABRT there.  Do I need to re-add the reference 
> there?  Is removing the array type REALLY what we wish to do?
>
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 10:09 AM
> To: Keane, Erich <erich.ke...@intel.com>; 
> reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
> cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
> guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
>
> Aaaahhh,
>
> Now I see. We have a parameter with the type that is a reference to VLA.
> I think in this case we should rework this code to something like this:
>
> if (ArgType->isVariablyModifiedType())
> ArgType =
> getContext().getCanonicalParamType(ArgType.getNonReferenceType());
>
>
> Try this solution.
>
> Best regards,
> Alexey Bataev
>
> On 10/10/2016 07:51 PM, Keane, Erich wrote:
>> I did.  I changed line 236 to "ArgType = 
>> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
>> suggesting), and the output was identical when doing dump on ArgType:
>>
>> This:
>>
>> LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>> `-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>>   |-BuiltinType 0xa3f4090 'int'
>>   `-<<>>
>> Vs:
>> LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>> `-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>>   |-BuiltinType 0xa3f4090 'int'
>>   `-<<>>
>>
>> -Original Message-
>> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
>> Sent: Monday, October 10, 2016 9:47 AM
>> To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
>> <erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
>> david.majne...@gmail.com; guy.ben...@intel.com
>> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
>> with OpenMP array access
>>
>> Hmm,
>>
>> I thought it must return a pointer to the element type rather than [*] kind 
>> type. Did you checked it?
>>
>> Best regards,
>> Alexey Bataev
>>
>> On 10/10/2016 07:09 PM, Erich Keane wrote:
>>> erichkeane added a comment.
>>>
>>> Andrey-
>>> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
>>> the same type in this case (at least in the reproduction).  I could 
>>> definitely change the call, though the changes to CGDebugInfo.cpp are 
>>> apparently also necessary even in that case.
>>>
>>> Is there additional logic that should happen in CGStmtOpenMP that I'm 
>>> missing?
>>>
>>>
>>> Repository:
>>>  rL LLVM
>>>
>>> https://reviews.llvm.org/D25373
>>>
>>>
>>>

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

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Keane, Erich via cfe-commits 
That does turn ArgType into a PointerType int*.  However, line 301 
(GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference type a 
bit later, resulting in a SIGABRT there.  Do I need to re-add the reference 
there?  Is removing the array type REALLY what we wish to do?

-Original Message-
From: Alexey Bataev [mailto:a.bat...@hotmail.com] 
Sent: Monday, October 10, 2016 10:09 AM
To: Keane, Erich <erich.ke...@intel.com>; 
reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
cfe-commits@lists.llvm.org; dblai...@gmail.com; david.majne...@gmail.com; 
guy.ben...@intel.com
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Aaaahhh,

Now I see. We have a parameter with the type that is a reference to VLA. 
I think in this case we should rework this code to something like this:

if (ArgType->isVariablyModifiedType())
   ArgType = 
getContext().getCanonicalParamType(ArgType.getNonReferenceType());


Try this solution.

Best regards,
Alexey Bataev

On 10/10/2016 07:51 PM, Keane, Erich wrote:
> I did.  I changed line 236 to "ArgType = 
> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
> suggesting), and the output was identical when doing dump on ArgType:
>
> This:
>
>LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>`-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>  |-BuiltinType 0xa3f4090 'int'
>  `-<<>>
> Vs:
>LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>`-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>  |-BuiltinType 0xa3f4090 'int'
>  `-<<>>
>
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 9:47 AM
> To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
> <erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
> david.majne...@gmail.com; guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
>
> Hmm,
>
> I thought it must return a pointer to the element type rather than [*] kind 
> type. Did you checked it?
>
> Best regards,
> Alexey Bataev
>
> On 10/10/2016 07:09 PM, Erich Keane wrote:
>> erichkeane added a comment.
>>
>> Andrey-
>> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
>> the same type in this case (at least in the reproduction).  I could 
>> definitely change the call, though the changes to CGDebugInfo.cpp are 
>> apparently also necessary even in that case.
>>
>> Is there additional logic that should happen in CGStmtOpenMP that I'm 
>> missing?
>>
>>
>> Repository:
>> rL LLVM
>>
>> https://reviews.llvm.org/D25373
>>
>>
>>

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

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
Aaaahhh,

Now I see. We have a parameter with the type that is a reference to VLA. 
I think in this case we should rework this code to something like this:

if (ArgType->isVariablyModifiedType())
   ArgType = 
getContext().getCanonicalParamType(ArgType.getNonReferenceType());


Try this solution.

Best regards,
Alexey Bataev

On 10/10/2016 07:51 PM, Keane, Erich wrote:
> I did.  I changed line 236 to "ArgType = 
> getContext().getCanonicalParamType(ArgType);" (as I believe you were 
> suggesting), and the output was identical when doing dump on ArgType:
>
> This:
>
>LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
>`-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
>  |-BuiltinType 0xa3f4090 'int'
>  `-<<>>
> Vs:
>LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
>`-VariableArrayType 0xa451650 'int [*]' variably_modified  *
>  |-BuiltinType 0xa3f4090 'int'
>  `-<<>>
>
> -Original Message-
> From: Alexey Bataev [mailto:a.bat...@hotmail.com]
> Sent: Monday, October 10, 2016 9:47 AM
> To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
> <erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
> david.majne...@gmail.com; guy.ben...@intel.com
> Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
> with OpenMP array access
>
> Hmm,
>
> I thought it must return a pointer to the element type rather than [*] kind 
> type. Did you checked it?
>
> Best regards,
> Alexey Bataev
>
> On 10/10/2016 07:09 PM, Erich Keane wrote:
>> erichkeane added a comment.
>>
>> Andrey-
>> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
>> the same type in this case (at least in the reproduction).  I could 
>> definitely change the call, though the changes to CGDebugInfo.cpp are 
>> apparently also necessary even in that case.
>>
>> Is there additional logic that should happen in CGStmtOpenMP that I'm 
>> missing?
>>
>>
>> Repository:
>> rL LLVM
>>
>> https://reviews.llvm.org/D25373
>>
>>
>>

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

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Keane, Erich via cfe-commits 
I did.  I changed line 236 to "ArgType = 
getContext().getCanonicalParamType(ArgType);" (as I believe you were 
suggesting), and the output was identical when doing dump on ArgType:

This:

  LValueReferenceType 0xa451620 'int (&)[*]' variably_modified
  `-VariableArrayType 0xa4515e0 'int [*]' variably_modified  *
|-BuiltinType 0xa3f4090 'int'
`-<<>>
Vs:
  LValueReferenceType 0xa451690 'int (&)[*]' variably_modified
  `-VariableArrayType 0xa451650 'int [*]' variably_modified  *
|-BuiltinType 0xa3f4090 'int'
`-<<>>

-Original Message-
From: Alexey Bataev [mailto:a.bat...@hotmail.com] 
Sent: Monday, October 10, 2016 9:47 AM
To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
<erich.ke...@intel.com>; cfe-commits@lists.llvm.org; dblai...@gmail.com; 
david.majne...@gmail.com; guy.ben...@intel.com
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Hmm,

I thought it must return a pointer to the element type rather than [*] kind 
type. Did you checked it?

Best regards,
Alexey Bataev

On 10/10/2016 07:09 PM, Erich Keane wrote:
> erichkeane added a comment.
>
> Andrey-
> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
> the same type in this case (at least in the reproduction).  I could 
> definitely change the call, though the changes to CGDebugInfo.cpp are 
> apparently also necessary even in that case.
>
> Is there additional logic that should happen in CGStmtOpenMP that I'm missing?
>
>
> Repository:
>rL LLVM
>
> https://reviews.llvm.org/D25373
>
>
>

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

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
Hmm,

I thought it must return a pointer to the element type rather than [*] 
kind type. Did you checked it?

Best regards,
Alexey Bataev

On 10/10/2016 07:09 PM, Erich Keane wrote:
> erichkeane added a comment.
>
> Andrey-
> It seems that getVariableArrayDecayedType and getCanonicalParamType return 
> the same type in this case (at least in the reproduction).  I could 
> definitely change the call, though the changes to CGDebugInfo.cpp are 
> apparently also necessary even in that case.
>
> Is there additional logic that should happen in CGStmtOpenMP that I'm missing?
>
>
> Repository:
>rL LLVM
>
> https://reviews.llvm.org/D25373
>
>
>

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

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Erich Keane via cfe-commits 
erichkeane added a comment.

Andrey-
It seems that getVariableArrayDecayedType and getCanonicalParamType return the 
same type in this case (at least in the reproduction).  I could definitely 
change the call, though the changes to CGDebugInfo.cpp are apparently also 
necessary even in that case.

Is there additional logic that should happen in CGStmtOpenMP that I'm missing?


Repository:
  rL LLVM

https://reviews.llvm.org/D25373



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

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-10 Thread Alexey Bataev via cfe-commits 
ABataev added a comment.

I think the fix is not quite correct. I believe it's better to replace a call 
of getVariableArrayDecayedType() in CGStmtOpenMP.cpp by a call to 
getCanonicalParamType(). Try to replace it and check the result.


Repository:
  rL LLVM

https://reviews.llvm.org/D25373



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

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-07 Thread Keane, Erich via cfe-commits 
Ok, I dug into this deeper.  ASTContext.cpp:2811 (getVariableArrayDecayedType) 
intentionaly sets size to nullptr in this case for the purpose of turning it 
into a [*] type.  OpenMP.cpp:236 
(CodeGenFunction::GenerateOpenMPCapturedStmtFunction) calls this to replace 
variably modified type with this one.  It definitely looks like this is on 
purpose as far as I can tell.



From: Keane, Erich
Sent: Friday, October 7, 2016 11:56 AM
To: 'David Blaikie' <dblai...@gmail.com>; 
reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; 
cfe-commits@lists.llvm.org; david.majne...@gmail.com; 'Alexey Bataev' 
<a.bat...@hotmail.com>
Cc: junb...@codeaurora.org
Subject: RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Added Alexey to the list, he’s the OMP Maintainer, so hopefully he knows better 
☺

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Friday, October 7, 2016 11:51 AM
To: 
reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org<mailto:reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org>;
 Keane, Erich <erich.ke...@intel.com<mailto:erich.ke...@intel.com>>; 
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>; 
david.majne...@gmail.com<mailto:david.majne...@gmail.com>; 
guy.ben...@intel.com<mailto:guy.ben...@intel.com>
Cc: junb...@codeaurora.org<mailto:junb...@codeaurora.org>
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Could you explain how/why there's a null size expr? I would've thought it must 
have /some/ size for code generation purposes...

On Fri, Oct 7, 2016 at 11:33 AM Erich Keane 
<erich.ke...@intel.com<mailto:erich.ke...@intel.com>> wrote:
erichkeane created this revision.
erichkeane added reviewers: cfe-commits, dblaikie, majnemer, gbenyei.
erichkeane set the repository for this revision to rL LLVM.

OpenMP creates a variable array type with a a null size-expr.  The Debug 
generation failed to properly consider this case.  This patch adds a null check 
to prevent a null dereference seg-fault in this case, plus adds a test.


Repository:
  rL LLVM

https://reviews.llvm.org/D25373

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-openmp-array.cpp


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2181,7 +2181,8 @@
   Count = CAT->getSize().getZExtValue();
 else if (const auto *VAT = dyn_cast(Ty)) {
   llvm::APSInt V;
-  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
+  if (VAT->getSizeExpr() &&
+  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
 Count = V.getExtValue();
 }

Index: test/CodeGenCXX/debug-info-openmp-array.cpp
===
--- test/CodeGenCXX/debug-info-openmp-array.cpp
+++ test/CodeGenCXX/debug-info-openmp-array.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g -O0 -S 
-emit-llvm %s -o - | FileCheck %s
+
+
+void f(int m) {
+  int i;
+  int cen[m];
+#pragma omp parallel for
+  for (i = 0; i < m; ++i) {
+cen[i] = i;
+  }
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_array_type,
+// CHECK-NOT: size:
+// CHECK-SAME: align: 32
+// CHECK-SAME:  elements: [[ELEM_TYPE:![0-9]+]]
+// CHECK: !DISubrange(count: -1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-07 Thread Keane, Erich via cfe-commits 
Added Alexey to the list, he’s the OMP Maintainer, so hopefully he knows better 
☺

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Friday, October 7, 2016 11:51 AM
To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
<erich.ke...@intel.com>; cfe-commits@lists.llvm.org; david.majne...@gmail.com; 
guy.ben...@intel.com
Cc: junb...@codeaurora.org
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Could you explain how/why there's a null size expr? I would've thought it must 
have /some/ size for code generation purposes...

On Fri, Oct 7, 2016 at 11:33 AM Erich Keane 
<erich.ke...@intel.com<mailto:erich.ke...@intel.com>> wrote:
erichkeane created this revision.
erichkeane added reviewers: cfe-commits, dblaikie, majnemer, gbenyei.
erichkeane set the repository for this revision to rL LLVM.

OpenMP creates a variable array type with a a null size-expr.  The Debug 
generation failed to properly consider this case.  This patch adds a null check 
to prevent a null dereference seg-fault in this case, plus adds a test.


Repository:
  rL LLVM

https://reviews.llvm.org/D25373

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-openmp-array.cpp


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2181,7 +2181,8 @@
   Count = CAT->getSize().getZExtValue();
 else if (const auto *VAT = dyn_cast(Ty)) {
   llvm::APSInt V;
-  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
+  if (VAT->getSizeExpr() &&
+  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
 Count = V.getExtValue();
 }

Index: test/CodeGenCXX/debug-info-openmp-array.cpp
===
--- test/CodeGenCXX/debug-info-openmp-array.cpp
+++ test/CodeGenCXX/debug-info-openmp-array.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g -O0 -S 
-emit-llvm %s -o - | FileCheck %s
+
+
+void f(int m) {
+  int i;
+  int cen[m];
+#pragma omp parallel for
+  for (i = 0; i < m; ++i) {
+cen[i] = i;
+  }
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_array_type,
+// CHECK-NOT: size:
+// CHECK-SAME: align: 32
+// CHECK-SAME:  elements: [[ELEM_TYPE:![0-9]+]]
+// CHECK: !DISubrange(count: -1)

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

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-07 Thread Keane, Erich via cfe-commits 
I cannot personally right now, though I will dig into OpenMP implementation to 
see how intentional that is.  The size is seemingly determined by the OMP code 
generation as far as I can tell.

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Friday, October 7, 2016 11:51 AM
To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich 
<erich.ke...@intel.com>; cfe-commits@lists.llvm.org; david.majne...@gmail.com; 
guy.ben...@intel.com
Cc: junb...@codeaurora.org
Subject: Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference 
with OpenMP array access

Could you explain how/why there's a null size expr? I would've thought it must 
have /some/ size for code generation purposes...

On Fri, Oct 7, 2016 at 11:33 AM Erich Keane 
<erich.ke...@intel.com<mailto:erich.ke...@intel.com>> wrote:
erichkeane created this revision.
erichkeane added reviewers: cfe-commits, dblaikie, majnemer, gbenyei.
erichkeane set the repository for this revision to rL LLVM.

OpenMP creates a variable array type with a a null size-expr.  The Debug 
generation failed to properly consider this case.  This patch adds a null check 
to prevent a null dereference seg-fault in this case, plus adds a test.


Repository:
  rL LLVM

https://reviews.llvm.org/D25373

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-openmp-array.cpp


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2181,7 +2181,8 @@
   Count = CAT->getSize().getZExtValue();
 else if (const auto *VAT = dyn_cast(Ty)) {
   llvm::APSInt V;
-  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
+  if (VAT->getSizeExpr() &&
+  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
 Count = V.getExtValue();
 }

Index: test/CodeGenCXX/debug-info-openmp-array.cpp
===
--- test/CodeGenCXX/debug-info-openmp-array.cpp
+++ test/CodeGenCXX/debug-info-openmp-array.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g -O0 -S 
-emit-llvm %s -o - | FileCheck %s
+
+
+void f(int m) {
+  int i;
+  int cen[m];
+#pragma omp parallel for
+  for (i = 0; i < m; ++i) {
+cen[i] = i;
+  }
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_array_type,
+// CHECK-NOT: size:
+// CHECK-SAME: align: 32
+// CHECK-SAME:  elements: [[ELEM_TYPE:![0-9]+]]
+// CHECK: !DISubrange(count: -1)

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

Re: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-07 Thread David Blaikie via cfe-commits 
Could you explain how/why there's a null size expr? I would've thought it
must have /some/ size for code generation purposes...

On Fri, Oct 7, 2016 at 11:33 AM Erich Keane  wrote:

> erichkeane created this revision.
> erichkeane added reviewers: cfe-commits, dblaikie, majnemer, gbenyei.
> erichkeane set the repository for this revision to rL LLVM.
>
> OpenMP creates a variable array type with a a null size-expr.  The Debug
> generation failed to properly consider this case.  This patch adds a null
> check to prevent a null dereference seg-fault in this case, plus adds a
> test.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D25373
>
> Files:
>   lib/CodeGen/CGDebugInfo.cpp
>   test/CodeGenCXX/debug-info-openmp-array.cpp
>
>
> Index: lib/CodeGen/CGDebugInfo.cpp
> ===
> --- lib/CodeGen/CGDebugInfo.cpp
> +++ lib/CodeGen/CGDebugInfo.cpp
> @@ -2181,7 +2181,8 @@
>Count = CAT->getSize().getZExtValue();
>  else if (const auto *VAT = dyn_cast(Ty)) {
>llvm::APSInt V;
> -  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
> +  if (VAT->getSizeExpr() &&
> +  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
>  Count = V.getExtValue();
>  }
>
> Index: test/CodeGenCXX/debug-info-openmp-array.cpp
> ===
> --- test/CodeGenCXX/debug-info-openmp-array.cpp
> +++ test/CodeGenCXX/debug-info-openmp-array.cpp
> @@ -0,0 +1,17 @@
> +// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g
> -O0 -S -emit-llvm %s -o - | FileCheck %s
> +
> +
> +void f(int m) {
> +  int i;
> +  int cen[m];
> +#pragma omp parallel for
> +  for (i = 0; i < m; ++i) {
> +cen[i] = i;
> +  }
> +}
> +
> +// CHECK: !DICompositeType(tag: DW_TAG_array_type,
> +// CHECK-NOT: size:
> +// CHECK-SAME: align: 32
> +// CHECK-SAME:  elements:
> [[ELEM_TYPE:![0-9]+]]
> +// CHECK: !DISubrange(count: -1)
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

2016-10-07 Thread Erich Keane via cfe-commits 
erichkeane created this revision.
erichkeane added reviewers: cfe-commits, dblaikie, majnemer, gbenyei.
erichkeane set the repository for this revision to rL LLVM.

OpenMP creates a variable array type with a a null size-expr.  The Debug 
generation failed to properly consider this case.  This patch adds a null check 
to prevent a null dereference seg-fault in this case, plus adds a test.


Repository:
  rL LLVM

https://reviews.llvm.org/D25373

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-openmp-array.cpp


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2181,7 +2181,8 @@
   Count = CAT->getSize().getZExtValue();
 else if (const auto *VAT = dyn_cast(Ty)) {
   llvm::APSInt V;
-  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
+  if (VAT->getSizeExpr() &&
+  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
 Count = V.getExtValue();
 }
 
Index: test/CodeGenCXX/debug-info-openmp-array.cpp
===
--- test/CodeGenCXX/debug-info-openmp-array.cpp
+++ test/CodeGenCXX/debug-info-openmp-array.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g -O0 -S 
-emit-llvm %s -o - | FileCheck %s
+
+
+void f(int m) {
+  int i;
+  int cen[m];
+#pragma omp parallel for
+  for (i = 0; i < m; ++i) {
+cen[i] = i;
+  }
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_array_type,
+// CHECK-NOT: size:
+// CHECK-SAME: align: 32
+// CHECK-SAME:  elements: [[ELEM_TYPE:![0-9]+]]
+// CHECK: !DISubrange(count: -1)


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2181,7 +2181,8 @@
   Count = CAT->getSize().getZExtValue();
 else if (const auto *VAT = dyn_cast(Ty)) {
   llvm::APSInt V;
-  if (VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
+  if (VAT->getSizeExpr() &&
+  VAT->getSizeExpr()->EvaluateAsInt(V, CGM.getContext()))
 Count = V.getExtValue();
 }
 
Index: test/CodeGenCXX/debug-info-openmp-array.cpp
===
--- test/CodeGenCXX/debug-info-openmp-array.cpp
+++ test/CodeGenCXX/debug-info-openmp-array.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -target x86_64-unknown-unknown -fverbose-asm -fopenmp -g -O0 -S -emit-llvm %s -o - | FileCheck %s
+
+
+void f(int m) {
+  int i;
+  int cen[m];
+#pragma omp parallel for
+  for (i = 0; i < m; ++i) {
+cen[i] = i;
+  }
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_array_type,
+// CHECK-NOT: size:
+// CHECK-SAME: align: 32
+// CHECK-SAME:  elements: [[ELEM_TYPE:![0-9]+]]
+// CHECK: !DISubrange(count: -1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits