steakhal added a comment.

In D158499#4606337 <https://reviews.llvm.org/D158499#4606337>, @danix800 wrote:

> In D158499#4606291 <https://reviews.llvm.org/D158499#4606291>, @steakhal 
> wrote:
>
>> Thanks for submitting this.
>> A funny thing is that in my free time I was also working on this last week. 
>> I'll have a look at this more in depth during the week.
>> For the mean time here is my version, committed pretty much a couple days 
>> ago to my fork.
>> https://github.com/steakhal/llvm-project/commit/986059a146e036ec84db64868a079d3c4a0e5e16
>>
>> EDIT: fix the link to point to my fork.
>>
>> Your proposed change looks pretty similar to mine.
>
> Are you going to submit your changes? Which one do you think is more suitable?
>
> Please let me know whether I should continue on this one or wait for yours. 
> Thanks!

I think the difference between yours and my implementation is that I try to 
accept FAM like arrays even if they are nested inside other strict, if they are 
notionally the last field of the whole thing. I'm doing it because that is how 
the machines work, thus the model should model that and return the right extent 
even for those cases.
What I can tell, that I didn't check my implementation with the packed 
attribute. I assume it should work.
The decl has a tempting method checking if its a FAM, however that does not 
accept FAMs of size 1 or of any size.

So its hard to tell right away which approach is better, but I'll look into 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to