[PATCH] [RFC] __builtin___clear_cache support in LLVM
renato.golin at linaro.org
Fri Mar 14 15:46:00 PDT 2014
Comment at: include/llvm/Target/TargetLowering.h:2114
@@ +2113,3 @@
+ return 0;
JF Bastien wrote:
> Renato Golin wrote:
> > JF Bastien wrote:
> > > This seems like a pretty bad silent failure waiting to occur for targets other than x86/ARM/MIPS: implementor won't know that they have to add this. Making the function pure virtual would force everyone to return 0 when, like x86, there's nothing to do, and in all other cases they'd need to do the right thing for their platform. It's either break everyone's build loudly, or everyone's runtime silently.
> > >
> > > It would also help understanding if the comment said that return 0 means "call nothing" (unless I misunderstood and that's not what it means).
> > good point, I'll change that to llvm_unreachable or something of the sort.
> Pure virtual function is better, since it forces implementation. llvm_unreachable makes it a runtime error, albeit a better one than silently dropping the call!
Making it pure virtual means I have to implement them now, on *all* architectures, but I can't possibly know what's the expected behaviour in all of them, if it even makes sense on some of them. I'd probably end up implementing them as llvm_unreachable anyway.
llvm_unreachable in the base class is a pattern that is well used elsewhere in the same file and on similar situations and it's easy enough to report errors in that way (and to implement them later).
More information about the llvm-commits