zahiraam wrote:
> > > > > > How does this interact with #107598?
> > > > >
> > > > >
> > > > > Though this also changes things to ensure when TBAA data is set, it's
> > > > > always set on the call.
> > > >
> > > >
> > > > Wasn't already doing that? (setting the TBAA on the call?).
> > >
> > >
> > > It was setting it on `cast<llvm::Instruction>(Call.getScalarVal())` not
> > > necessarily the call (which you can get via an output on `EmitCall()`).
> > > At least in this case that meant it was putting the TBAA metadata on the
> > > `load x86_fp80` after the call. I'm not sure if there's other cases where
> > > something similar could happen.
> >
> >
> > > > > > How does this interact with #107598?
> > > > >
> > > > >
> > > > > Though this also changes things to ensure when TBAA data is set, it's
> > > > > always set on the call.
> > > >
> > > >
> > > > Wasn't already doing that? (setting the TBAA on the call?).
> > >
> > >
> > > It was setting it on `cast<llvm::Instruction>(Call.getScalarVal())` not
> > > necessarily the call (which you can get via an output on `EmitCall()`).
> > > At least in this case that meant it was putting the TBAA metadata on the
> > > `load x86_fp80` after the call. I'm not sure if there's other cases where
> > > something similar could happen.
> >
> >
> > Without this patch and without (#107598) the function `pow` doesn't
> > generate `int TBAA` info on the call, but it does on a call to `cargl` with
> > `-triple aarch64-unknown-unknown`.
> > `# | %call = tail call fp128 @cargl([2 x fp128] noundef alignstack(16)
> > undef) #1, !tbaa !2`
>
> I'm not sure I follow, but the issue I spotted was "int" TBAA metadata was
> being set on the load following the `pow` call, but it has the same root
> cause as the `cargl` issue.
>
> See the diff from the first commit:
>
> ```diff
> diff --git a/clang/test/CodeGen/math-libcalls-tbaa-indirect-args.c
> b/clang/test/CodeGen/math-libcalls-tbaa-indirect-args.c
> index dd013dcc8b3ca8..56c6b3ec00bc7e 100644
> --- a/clang/test/CodeGen/math-libcalls-tbaa-indirect-args.c
> +++ b/clang/test/CodeGen/math-libcalls-tbaa-indirect-args.c
> @@ -19,7 +19,7 @@ long double powl(long double a, long double b);
> // CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 16, ptr nonnull
> [[BYVAL_TEMP1]]) #[[ATTR3]]
> // CHECK-NEXT: store x86_fp80 0xK40008000000000000000, ptr
> [[BYVAL_TEMP1]], align 16, !tbaa [[TBAA3]]
> // CHECK-NEXT: call void @powl(ptr dead_on_unwind nonnull writable
> sret(x86_fp80) align 16 [[TMP]], ptr noundef nonnull [[BYVAL_TEMP]], ptr
> noundef nonnull [[BYVAL_TEMP1]]) #[[ATTR3]]
> -// CHECK-NEXT: [[TMP0:%.*]] = load x86_fp80, ptr [[TMP]], align 16, !tbaa
> [[TBAA7:![0-9]+]]
> +// CHECK-NEXT: [[TMP0:%.*]] = load x86_fp80, ptr [[TMP]], align 16, !tbaa
> [[TBAA3]]
> // CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 16, ptr nonnull
> [[BYVAL_TEMP]]) #[[ATTR3]]
> // CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 16, ptr nonnull
> [[BYVAL_TEMP1]]) #[[ATTR3]]
> // CHECK-NEXT: store x86_fp80 [[TMP0]], ptr [[AGG_RESULT]], align 16,
> !tbaa [[TBAA3]]
> @@ -33,6 +33,4 @@ long double test_powl() {
> // CHECK: [[META4]] = !{!"long double", [[META5:![0-9]+]], i64 0}
> // CHECK: [[META5]] = !{!"omnipotent char", [[META6:![0-9]+]], i64 0}
> // CHECK: [[META6]] = !{!"Simple C/C++ TBAA"}
> -// CHECK: [[TBAA7]] = !{[[META8:![0-9]+]], [[META8]], i64 0}
> -// CHECK: [[META8]] = !{!"int", [[META5]], i64 0}
> //.
> ```
>
> > but it does on a call to cargl with `-triple aarch64-unknown-unknown`.
> > `> `# | %call = tail call fp128 @cargl([2 x fp128] noundef alignstack(16)
> > undef) #1, !tbaa !2`
>
> That looks okay though? It's not passing or returning values via pointers, so
> it should be safe to set the "int" TBAA (which indicates the only pointer it
> could read is errno).
Not really! `int TBAA` in in our downstream compiler is interpreted as
describing the function arguments (they are not int) and the `load/store` of
the argument before the library call are begin eliminated which result in
unexpected behavior.
I am not objecting to anything; I am just wondering what difference it makes to
have the TBAA attached to the call instead of how it is now.
https://github.com/llvm/llvm-project/pull/108853
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
