[PATCH] D115409: [SelectionDAGBuilder] drop special handling for CallBr

James Y Knight via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 13 14:44:16 PST 2021


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



More information about the cfe-commits mailing list