zahiraam added a comment. In D151834#4644422 <https://reviews.llvm.org/D151834#4644422>, @zahiraam wrote:
> In D151834#4644394 <https://reviews.llvm.org/D151834#4644394>, @aaron.ballman > wrote: > >> In D151834#4644391 <https://reviews.llvm.org/D151834#4644391>, @zahiraam >> wrote: >> >>> In D151834#4644378 <https://reviews.llvm.org/D151834#4644378>, >>> @aaron.ballman wrote: >>> >>>> In D151834#4644375 <https://reviews.llvm.org/D151834#4644375>, @zahiraam >>>> wrote: >>>> >>>>> In D151834#4644373 <https://reviews.llvm.org/D151834#4644373>, >>>>> @aaron.ballman wrote: >>>>> >>>>>> In D151834#4643925 <https://reviews.llvm.org/D151834#4643925>, @uabelho >>>>>> wrote: >>>>>> >>>>>>> Hi @zahiraam , >>>>>>> >>>>>>> I have a couple of downstream testcases that fail with this patch. >>>>>>> Before >>>>>>> >>>>>>> > clang bbi-86364.c -lm -O3 >>>>>>> > ./a.out >>>>>>> >>>>>>> passed but with the patch the assert in the program fails: >>>>>>> >>>>>>> a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) >>>>>>> == 33' failed. >>>>>>> >>>>>>> Is this as expected? >>>>>>> >>>>>>> F29200339: bbi-86364.c <https://reviews.llvm.org/F29200339> >>>>>> >>>>>> This seems unexpected to me and it seems to relate to whether you >>>>>> include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do >>>>>> you have ideas as to what's going on? >>>>> >>>>> I haven't looked at it as I saw that the comment has been deleted. Let me >>>>> look into it. >>>> >>>> Oh, it's not the inclusion of errno.h that matters, it's the declaration >>>> of `__errno_location`: https://godbolt.org/z/zo4PaPEME -- it seems that >>>> inclusion of `__attribute__ ((__const__))` is what makes the distinction: >>>> https://godbolt.org/z/1bePhvaG4 >>> >>> Yes confirming this. In the errno.h header the function is declared: extern >>> int *__errno_location (void) __attribute__ ((__nothrow__ )) __attribute__ >>> ((__const__)); >>> Since we can't really change the header, this needs to be changed in the >>> compiler? >> >> Yeah, I think this is a bug with your patch, but I'm not certain where. I >> suspect the changes to CGBuiltin.cpp are what are causing this issue -- CC >> @efriedma and @rjmccall as they might have ideas. > > I think the issue comes from here: > https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365 This looks like it's happening only for -Oi (i>=1) and on Linux only? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.org/D151834 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
