[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D52191#1238648, @srhines wrote:

> In https://reviews.llvm.org/D52191#1238628, @dblaikie wrote:
>
> > Sure, looks good. Though my other/vague concern is why does this case error about fomit-frame-pointer having no effect, but other things (like using -fomit-frame-pointer on a target that requires frame pointers) that ignore -fomit-frame-pointer don't? Weird. But it probably makes sense somehow.
>
>
> I think the original issue is that one should not **explicitly** say "omit frame pointers" and "instrument for profiling with mcount". It's possible that there should be other errors for using "omit frame pointers" on targets that require them, but that should be checked independently elsewhere. Do we have many (any) of these kinds of targets? I'm going to submit this patch, but am happy to add a diagnostic later on for the other case, if you think that is worthwhile.


Yeah, looks like the other cases (targets, specifically - and yeah, I didn't look at the implementation of shouldUseFramePointer far enough to see which targets, etc - OK, I just looked now, and it's Darwin ARM & Thumb - so arguably on those targets, since -fomit-frame-pointer has no effect, maybe it's not important to error on -fomit-frame-pointer -pg)

Also I just realized you probably want/need "!shouldUseFramePointer" on your error. (missed the inversion) - sorry I didn't catch that in review. Should catch that with your test from the previous change?


Repository:
  rL LLVM

https://reviews.llvm.org/D52191