jyknight added a comment. OK, I do think that special case definitely needs to be deleted. It's assuming that the block args are in a particular place in the argument list of the callbr -- the place Clang put them -- and then assigned special behavior based on that location. But I think it shouldn't do so -- they should be able to placed anywhere in the arglist.
The question then, is why this code needs to exist? It's accomplishing two things: 1. If we get rid of this code but continue to use the "X" constraint, llvm won't emit an immediate symbol reference (today), rather, it will emit a register reference instead. Even though X means "accept anything", and therefore a register is thus supposedly acceptable, it seems like it should probably prefer emitting an immediate if it can. (GCC does emit it as an immediate here) 2. Emits a TargetBlockAddress instead of BlockAddress. (I'm not sure what the use of TargetBlockAddress accomplishes? I _think_ it should be okay to use BlockAddress instead, but I'm not 100% sure...) Assuming the point 2 is OK, then I think ideally, we'd flip the order of things, and do: a. Cause "X" constraint to emit an immediate symbol reference if it can, rather than copying to a register first -- and remove this special case. (Fixes bug.) b. Updates tests and change clang to emit "i" instead of "X" constraints. (Clarifies intent that an immediate is actually required). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115409/new/ https://reviews.llvm.org/D115409 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
