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' >>> `-<<<NULL>>> >>> Vs: >>> LValueReferenceType 0xa451690 'int (&)[*]' variably_modified >>> `-VariableArrayType 0xa451650 'int [*]' variably_modified * >>> |-BuiltinType 0xa3f4090 'int' >>> `-<<<NULL>>> >>> >>> -----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
