mehdi_amini added inline comments.
================
Comment at: test/CodeGenCXX/return.cpp:21
+ // CHECK-NOSTRICT-NEXT: load
+ // CHECK-NOSTRICT-NEXT: ret i32
+ // CHECK-NOSTRICT-NEXT: }
----------------
rjmccall wrote:
> mehdi_amini wrote:
> > Quuxplusone wrote:
> > > Can you explain why a load from an uninitialized stack location would be
> > > *better* than a trap and/or `unreachable`? IIUC this is basically what
> > > Mehdi is asking: i.e., can you explain the rationale for this patch,
> > > because I don't get it either. It *seems* like a strict regression in
> > > code quality.
> > There is a difference. LLVM will optimize:
> >
> > ```
> > define i32 @foo() {
> > %1 = alloca i32, align 4
> > %2 = load i32, i32* %1, align 4
> > ret i32 %2
> > }
> > ```
> >
> > to:
> >
> > ```
> > define i32 @foo() {
> > ret i32 undef
> > }
> > ```
> >
> > Which means "return an undefined value" (basically any valide bit-pattern
> > for an i32). This is not undefined behavior if the caller does not use the
> > value with a side-effect.
> >
> > However with:
> >
> > ```
> > define i32 @foo() {
> > unreachable
> > }
> > ```
> >
> > Just calling `foo()` is undefined behavior, even if the returned value
> > isn't used.
> >
> > So what this patch does is actually making the compiled-code `safer` by
> > inhibiting some optimizations based on this UB.
> We've been disabling this optimization completely in Apple compilers for
> years, so allowing it to ever kick in is actually an improvement. I'll try
> to elaborate on our reasoning for this change, which *is* actually somewhat
> Apple-specific.
>
> Falling off the end of a non-void function is only undefined behavior in C++,
> not in C. It is fairly common practice to compile C code as C++, and while
> there's a large number of potential language incompatibilities there, they
> tend to be rare or trivial to fix in practice. At Apple, we have a specific
> need to compile C code as C++ because of the C++-based IOKit API: while
> drivers are overwhelmingly written as C code, at Apple they have to be
> compiled as C++ in order to talk to the kernel. Moreover, often Apple did
> not write the drivers in question, and maintaining a large set of patches in
> order to eliminate undefined behavior that wasn't actually UB in the
> originally-intended build configuration is not really seen as acceptable.
>
> It makes sense to me to expose this as a flag akin to -fno-strict-aliasing in
> order to support C-in-C++ code bases like Apple's drivers. It is possible
> that we don't actually have to change the default for the flag on Apple
> platforms and can instead pursue more targeted build changes.
I totally understand why such flag is desirable, it is just not a clear cut to
make it the default on one platform only (and having this flag available
upstream does not prevent the Xcode clang from enabling this by default though,
if fixing the build settings isn't possible/desirable).
Repository:
rL LLVM
https://reviews.llvm.org/D27163
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
