Thanks for fixing the unit test.

Looks good to me.

You are correct. I have pushed https://reviews.llvm.org/D53451 to rectify.

I was not able to formulate a unit test that exposes this issue.
I figured this problem when I was working on a DAG mutation for the pipeliner.

There is no test case associated with this patch as I have not found one yet.

Thanks for taking care of this

This patch is already merged in a different commit

Ping

Thanks for this fix

This patch is committed in r309445

Hans, I am not sure if this is the correct place and correct way to add the documentation. Let me know

Also it makes me sad that the attribute isn't documented anywhere. Would you like to look into that, possible as a follow-up patch?

Sure. I will push a follow up patch

We have decided to have this feature under "-fno-jump-tables" clang flag.
To this affect, this patch updates the code to control the generation of
lookup tables based on the attribute "no-jump-tables"

The only change that is needed is to disable lookup-tables based on the attribute "no-jump-tables" set by "-fno-jump-tables" clang flag.
It implies that this change is not required.

Refer to https://reviews.llvm.org/D35577 as we decided to disable lookup tables under -fno-jump-tables

Thanks. I will make change to this affect

I am waiting for others to approve/review the decision.

If it is okay for the reviewers, I have no problem using -fno-jump-tables to this effect.
I need to update https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579

"Should this just be part of the tuning for the hexagon backend and not options at all"

This will be useful to all the backends/archs that support a tightly coupled memory.
AFAIK, hexagon is not the only target that has a TCM.

This is not going to be a temporary option

The discussion is scattered across these patches https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579.
I will provide a brief summary here:

Moreover, why can't this determining factor be built into the compiler so the user doesn't even have to bother. That would be a more ideal user experience.

Here is a use case : For the code that stays in TCM, the customer doesn't want the data that the code refers to be outside of TCM. As kparzysz mentioned, the loads cause a huge latency and is not intended. Disabling table generation is the right thing to do here. For code that stays in regular memory, generating tables is far more efficient than a bunch of if-elses.

Wouldn't the fix be to make the backend deal with this, then? Either by putting the table with the function text, or or opting out of lookup tables? It seems that might be a better experience for the user.

That is perfectly reasonable and in fact i have committed a hexagon change recently to that effect . The llvm flag hexagon-emi-lookup-tables controls the generation of lookup table for hexagon.
The problem is, I don't want the users of the compiler to use a combination of front end and back end flags to get the desired result.
"-fno-jump-tables -mllvm -hexagon-emit-lookup-tables=false". This could be much neater with a "-fno-jump-tables -fno-lookup-tables" or better just "-fno-switch-tables"

It looks like TargetTransformInfo already has a hook, shouldBuildLookupTables(), for disabling this optimization. Why not just set that to false for Hexagon?

TCM is often small and you don't want to either enable or disable the table generation for a specific backend. As of today,
hexagon-emit-lookup-tables llvm flag control the generation of lookup tables for hexagon backend.
The real problem here is , I do not want the user to use a combination of front end and backend flags to get the desired result.

Made changes as per reviewers comments

Made the changes asked by reviewers

I will try to address the concerns here:

For backends with "tightly coupled memory", in scenarios where the data is far away from text pays a good amount of penalty in terms of latency.
Hexagon is one such backend. The tables (both lookup and jump) which are being generated are treated as globals with internal linkage and by default
will be placed in read only data.

The switch-case statements generate two kinds of tables.

1. Jump tables
2. Lookup tables.

This patch relies on https://reviews.llvm.org/D35577