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); + ---------------- ABataev wrote: > doru1004 wrote: > > doru1004 wrote: > > > ABataev wrote: > > > > doru1004 wrote: > > > > > 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. > > > > Can you check that the declaration is not captured in the target > > > > context? If it is not captured, it is declared in the target region and > > > > should be emitted as delayed. > > > How do I check that? There doesn't seem to be a list of captured > > > variables available at that point in the code. > > > > > So the complication is that the same declaration is captured and not > > captured at the same time. It can be declared inside a teams distribute > > (not captured) but captured by an inner parallel for (captured). I think I > > can come up with something though. > Need to check the captures in the target regions only I cannot get a handle on the target directive in markedAsEscaped function in order to look at its captures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153883/new/ https://reviews.llvm.org/D153883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
