tberghammer added a comment.

UINT32_MAX is a kind of a random value what is most likely won't be used on any 
architecture as a condition code (I can't imagine having so much different 
condition flags) and my current intention is to map the unconditional value to 
into it as it is already done on ARM where the unconditional value is 0b1110 
and 0b1111.

I don't really like the idea of introducing a new GetUnconditionalCondition() 
method because I think it over complicates the API in a situation where it is 
not necessary. I would better prefer to create a static constant for the 
unconditional condition (with setting it to UINT32_MAX) so the implementation 
is a bit cleaner. If we really want to make a very clean interface then I would 
suggest to introduce a new "Condition" class with a virtual equality operator 
and a virtual isUnconditional function and then we can make 
GetInstructionCondition return a pointer to an instance so we don't restrict 
the storage format for the architectures to a uint32_t. I think this would be 
the most general API but I also feel it would be a massive over-engineering.

For llvm::Optional<T> the main issue is that (non) equality operator is 
intentionally not defined for llvm::Optional<T> (I don't know why) what would 
make UnwindAssemblyInstEmulation.cpp:161 very strange. Also debugging code 
containing llvm data types is generally pretty complicated.

All in all I would suggest to use a uint32_t/uint64_t for the condition code 
with UINT32_MAX/UINT64_MAX meaning unconditional and I propose the introduction 
of a new public/protected static constant on EmulateInstruction what represents 
the unconditional value. This way the implementation is cleaner because will be 
obvious when we are comparing against an unconditional condition with keeping 
the API simple.

What do you think?


http://reviews.llvm.org/D16814



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to