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
