rjmccall added inline comments.
================
Comment at: test/CodeGenCXX/return.cpp:21
+ // CHECK-NOSTRICT-NEXT: load
+ // CHECK-NOSTRICT-NEXT: ret i32
+ // CHECK-NOSTRICT-NEXT: }
----------------
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.
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
