[PATCH] D153883: [Clang][OpenMP] Delay emission of __kmpc_alloc_shared for escaped VLAs

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:2806
+  /// Return true if all the emissions for the VLA size have occured.
+  bool hasVLASize(const VariableArrayType *type);
+
----------------
doru1004 wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > doru1004 wrote:
> > > > ABataev wrote:
> > > > > doru1004 wrote:
> > > > > > ABataev wrote:
> > > > > > > doru1004 wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > 1. Is it possible that VariableArrayType does not have VLA size?
> > > > > > > > > 2. Fix param name
> > > > > > > > @ABataev How would point 1 happen?
> > > > > > > You're adding a function that checks if VLA type has VLA size. I'm asking, if it is possible for VLA type to not have VLA size at all? Why do you need this function?
> > > > > > This function checks if the expression of the size of the VLA has already been emitted and can be used.
> > > > > Why the emission of VLA size can be delayed?
> > > > Because the size of the VLA is emitted in the user code and the prolog of the function happens before that. The emission of the VLA needs to be delayed until its size has been emitted in the user code.
> > > This is very fragile approach. Can you try instead try to improve markAsEscaped function and fix insertion of VD to EscapedVariableLengthDecls and if the declaration is internal for the target region, insert it to DelayedVariableLengthDecls?
> > I am not sure what the condition would be, at that point, to choose between one list or the other. I'm not sure what you mean by the declaration being internal to the target region.
> Any thoughts? As far as I can tell all VLAs that reach that point belong in `DelayedVariableLengthDecls`
@ABataev I cannot think of a condition to use for the distinction in markedAsEscaped(). Could you please explain in more detail what you want me to check? I can make the rest of the changes happen no problem but I don't know what the condition is. Unless you tell me otherwise, I think the best condition is to check whether the VLA size has been emitted (i.e. that is is part of the VLASize list) in which case the code as is now is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153883