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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
