ABataev 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: > > > > 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 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
