paulkirth added a comment. In D158668#4611842 <https://reviews.llvm.org/D158668#4611842>, @MatzeB wrote:
>> My initial reaction to this was that we should keep the >> --unlikely-branch-weights flag available > > I don't feel strongly about it and can put it back. But can you give some > reasoning? I only see this flag having a real use to express small ratios > like 3:2 which doesn't really seem helpful to express "likely"/"unlikely"... I think the rest of the comment agrees w/ you that it probably isn't that useful :) My concern was testing, but given that you've barley had to update anything, I'd say that early feeling is just wrong, since we aren't using it except in a handful of cases. As such I think it can go away w/ only a short FYI about a planned change. In D158668#4611845 <https://reviews.llvm.org/D158668#4611845>, @MatzeB wrote: >> On the other hand, I dislike exposing some internal detail of a pass. > > I think the question to ask is whether a concept like "likely branch" or > "unlikely branch" shouldn't be more universal and not be a pass internal > detail? Otherwise it feels like my patches and other places would need to > insert new `llvm.expect` usages into the IR and add more instances of > `LowerExpect` pass into the pipeline to not risk pass ordering problems for > the small gain of having a more abstract representation... Again, I think the comments directly following agree w/ your position: > That said, it seems like these have more value when exposed, so I think it > would be fine, as long as we can prevent frontends from using the APIs, like > you mentioned in https://reviews.llvm.org/D158642#4611025. I think he fact that we're discussing this means it isn't functionally internal anymore, so that's maybe reason enough to do this. So to clarify, I'm broadly in favor of this direction. I'd also posit, that maybe since we're changing this we should reevaluate the numbers we use as defaults. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158668/new/ https://reviews.llvm.org/D158668 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
