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 (&)[*]' 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 >>>>> >>>>> >>>>> > > <build_results.txt> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
