owenpan added inline comments.
================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+ const auto &NextLine = *I[1];
+ const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+ if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)
----------------
HazardyKnusperkeks wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > owenpan wrote:
> > > > I would move this line to just before handling empty record blocks
> > > > below.
> > > I'd rather keep the definitions close together.
> > If it were just a simple initialization, it wouldn't matter much. However,
> > it would be a bit wasteful as `PreviousLine` always gets computed here even
> > if the function may return before the pointer would get chance to be used.
> > If you really want to keep all related definitions together, wouldn't you
> > want to move lines 214-215 up to right after line 211?
> In a release build I'm betting that the compiler is smart enough to never
> calculate `PreviousLine` and that performance doesn't really matter was shown
> in D116318.
>
> But yeah moving it up to `TheLine` is consistent and will do.
> In a release build I'm betting that the compiler is smart enough to never
> calculate `PreviousLine`
No compiler will (or should) skip generating code that calculates
`PreviousLine`, which may or may not get used at runtime. I'm not aware of any
compiler that is smart enough to move the initialization code to just before
where the variable is first used.
To illustrate the difference, I compiled the code below with `clang++ -Wall
-std=c++11 -O3 -S foo.cpp`:
```
#include <vector>
using namespace std;
using Type = vector<int *>;
int f(const Type &V, Type::const_iterator I) {
const auto *P = I != V.begin() ? I[-1] : nullptr;
if (I == V.end())
return 0;
return *P;
}
```
The generated code in foo.s includes the following:
```
.cfi_startproc
; %bb.0:
ldr x8, [x0]
cmp x8, x1
b.eq LBB0_3
; %bb.1:
ldur x8, [x1, #-8]
ldr x9, [x0, #8]
cmp x9, x1
b.eq LBB0_4
LBB0_2:
ldr w0, [x8]
ret
LBB0_3:
mov x8, #0
ldr x9, [x0, #8]
cmp x9, x1
b.ne LBB0_2
LBB0_4:
mov w0, #0
ret
.cfi_endproc
```
As you can see, the initialization code was neither optimized away nor moved.
When I changed the function body to:
```
if (I == V.end())
return 0;
const auto *P = I != V.begin() ? I[-1] : nullptr;
return *P;
```
The generated code became much simpler:
```
.cfi_startproc
; %bb.0:
ldr x8, [x0, #8]
cmp x8, x1
b.eq LBB0_2
; %bb.1:
ldur x8, [x1, #-8]
ldr w0, [x8]
ret
LBB0_2:
mov w0, #0
ret
.cfi_endproc
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115060/new/
https://reviews.llvm.org/D115060
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
